Re: [PATCH 13/14] iio: pressure: ms5611: Use devm_regulator_get_enable()

From: Jonathan Cameron <jic23@kernel.org>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: linux-iio@vger.kernel.org,
	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
	Cosmin Tanislav <cosmin.tanislav@analog.com>,
	Jagath Jog J <jagathjog1996@gmail.com>,
	Sean Nyekjaer <sean@geanix.com>,
	Dmitry Rokosov <DDRokosov@sberdevices.ru>,
	Linus Walleij <linus.walleij@linaro.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Michael Hennerich <michael.hennerich@analog.com>,
	Lorenzo Bianconi <lorenzo@kernel.org>,
	Martyn Welch <martyn.welch@collabora.com>,
	Gwendal Grignou <gwendal@chromium.org>,
	Stephen Boyd <swboyd@chromium.org>,
	Tomasz Duszynski <tduszyns@gmail.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: Re: [PATCH 13/14] iio: pressure: ms5611: Use devm_regulator_get_enable()
Date: Sun, 4 Dec 2022 18:19:10 +0000	[thread overview]
Message-ID: <20221204181910.1ee1ae92@jic23-huawei> (raw)
In-Reply-To: <e4dbfb4d-cd92-f72f-caf0-4eece7678de7@gmail.com>

On Mon, 17 Oct 2022 09:26:15 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 10/16/22 19:34, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > This driver only turns the power on at probe and off via a custom
> > devm_add_action_or_reset() callback.  
> 
> I think this one did not use devm_add_action_or_reset()
The other changes hitting this driver are now upstream so applied
to the togreg branch of iio.git with a bit of fuzz.

Thanks,

Jonathan

> 
> > The new devm_regulator_get_enable()
> > replaces this boilerplate code. Some additional refactoring to drop
> > now unnecessary unwinding after this change.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Cc: Tomasz Duszynski <tduszyns@gmail.com>
> > ---
> >   drivers/iio/pressure/ms5611.h      |  3 ---
> >   drivers/iio/pressure/ms5611_core.c | 32 +++++-------------------------
> >   2 files changed, 5 insertions(+), 30 deletions(-)
> >   
> 
> // snip
> 
> > @@ -477,7 +456,6 @@ void ms5611_remove(struct iio_dev *indio_dev)
> >   {
> >   	iio_device_unregister(indio_dev);
> >   	iio_triggered_buffer_cleanup(indio_dev);
> > -	ms5611_fini(indio_dev);
> >   }
> >   EXPORT_SYMBOL_NS(ms5611_remove, IIO_MS5611);  
> 
> Just a thought but maybe the whole remove() could be done using devm()? 
> (As far as I can say the current flow works. AFAICS the devm unwinding 
> is done after the remove() has been ran. But perhaps it would be cleaner 
> if the remove() would not need to be exported at all.)
> 
> With, or without the remove() removed, if commit description is fixed:
> Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>
>