Re: [PATCH v2 2/2] staging: iio: ad7780: moving ad7780 out of staging

[Renato Lui Geh]

On 01/28, Dan Carpenter wrote:
> On Sun, Jan 27, 2019 at 06:33:52PM -0200, Renato Lui Geh wrote:
> > +	st->reg = devm_regulator_get(&spi->dev, "avdd");
> > +	if (IS_ERR(st->reg))
> > +		return PTR_ERR(st->reg);
> > +
> > +	ret = regulator_enable(st->reg);
> > +	if (ret) {
> > +		dev_err(&spi->dev, "Failed to enable specified AVdd supply\n");
> > +		goto error_disable_reg;
> > +	}
>
> We don't need to disable the regulator if enabling it failed (probably,
> I haven't looked at the code).

I wasn't sure if ret would need to be cleaned up on failure, so I kept
it as it was before. I'll change it to a simple return if needed though.
> > +
> > +	ret = ad_sd_setup_buffer_and_trigger(indio_dev);
> > +	if (ret)
> > +		goto error_disable_reg;
> > +
> > +	ret = iio_device_register(indio_dev);
> > +	if (ret)
> > +		goto error_cleanup_buffer_and_trigger;
> > +
> > +	return 0;
> > +
> > +error_cleanup_buffer_and_trigger:
> > +	ad_sd_cleanup_buffer_and_trigger(indio_dev);
> > +error_disable_reg:
> > +	regulator_disable(st->reg);
> > +
>
> Well chosen label names, btw.  Very easy to review.

That was already there before I came. But yes, very nice choice of
names. :)
> > +	return ret;
> > +}
> > +
>
> regards,
> dan carpenter