Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sat, 20 Aug 2022 12:21:20 +0100
From:   Jonathan Cameron <jic23@kernel.org>
To:     Matti Vaittinen <mazziesaccount@gmail.com>
Cc:     Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>,
        Nuno =?UTF-8?B?U8Oh?= <nuno.sa@analog.com>,
        Lars-Peter Clausen <lars@metafoo.de>,
        linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 07/14] iio: ltc2688: Simplify using
 devm_regulator_*get_enable()
Message-ID: <20220820122120.57dddcab@jic23-huawei>
In-Reply-To: <a29493f594c84b3bd852e462bbd3e591a8575a27.1660934107.git.mazziesaccount@gmail.com>
References: <cover.1660934107.git.mazziesaccount@gmail.com>
        <a29493f594c84b3bd852e462bbd3e591a8575a27.1660934107.git.mazziesaccount@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 19 Aug 2022 22:19:17 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Use devm_regulator_bulk_get_enable() instead of open coded bulk-get,
> bulk-enable, add-action-to-disable-at-detach - pattern.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
> ---
> v2 => v3
> Split to own patch.
> ---
>  drivers/iio/dac/ltc2688.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/dac/ltc2688.c b/drivers/iio/dac/ltc2688.c
> index 28bdde2d3088..fcad3efe62ea 100644
> --- a/drivers/iio/dac/ltc2688.c
> +++ b/drivers/iio/dac/ltc2688.c
> @@ -84,7 +84,6 @@ struct ltc2688_chan {
>  struct ltc2688_state {
>  	struct spi_device *spi;
>  	struct regmap *regmap;
> -	struct regulator_bulk_data regulators[2];
>  	struct ltc2688_chan channels[LTC2688_DAC_CHANNELS];
>  	struct iio_chan_spec *iio_chan;
>  	/* lock to protect against multiple access to the device and shared data */
> @@ -902,13 +901,6 @@ static int ltc2688_setup(struct ltc2688_state *st, struct regulator *vref)
>  			       LTC2688_CONFIG_EXT_REF);
>  }
>  
> -static void ltc2688_disable_regulators(void *data)
> -{
> -	struct ltc2688_state *st = data;
> -
> -	regulator_bulk_disable(ARRAY_SIZE(st->regulators), st->regulators);
> -}
> -
>  static void ltc2688_disable_regulator(void *regulator)
>  {
>  	regulator_disable(regulator);
> @@ -970,6 +962,7 @@ static int ltc2688_probe(struct spi_device *spi)
>  	struct regulator *vref_reg;
>  	struct device *dev = &spi->dev;
>  	int ret;
> +	static const char * const regulators[] = {"vcc", "iovcc"};
trivial - slight preference for 
 { "vcc", "iovcc" };

This isn't as important as for numeric values as we get some readability
from the quotes but still nice to have.

For the whole static / vs non static. My personal preference is not
to have the static marking but I don't care that much.

>  
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (!indio_dev)
> @@ -988,21 +981,11 @@ static int ltc2688_probe(struct spi_device *spi)
>  		return dev_err_probe(dev, PTR_ERR(st->regmap),
>  				     "Failed to init regmap");
>  
> -	st->regulators[0].supply = "vcc";
> -	st->regulators[1].supply = "iovcc";
> -	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(st->regulators),
> -				      st->regulators);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "Failed to get regulators\n");
> -
> -	ret = regulator_bulk_enable(ARRAY_SIZE(st->regulators), st->regulators);
> +	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
> +					     regulators);
>  	if (ret)
>  		return dev_err_probe(dev, ret, "Failed to enable regulators\n");
>  
> -	ret = devm_add_action_or_reset(dev, ltc2688_disable_regulators, st);
> -	if (ret)
> -		return ret;
> -
>  	vref_reg = devm_regulator_get_optional(dev, "vref");
>  	if (IS_ERR(vref_reg)) {
>  		if (PTR_ERR(vref_reg) != -ENODEV)