Re: [PATCH v3 07/14] iio: ltc2688: Simplify using devm_regulator_*get_enable()

[Matti Vaittinen]

On 8/20/22 14:21, Jonathan Cameron wrote:
> On Fri, 19 Aug 2022 22:19:17 +0300
> Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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@xxxxxxxxx>
> >
> > ---
> > 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.

Right. I'll fix it.

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

I'd like to stick with the static here. I know this one particular array 
does not have much of a footprint - but I'd like to encourage the habit 
of considering the memory usage. This discussion serves as an example of 
how unknown the impact of making const data static is. I didn't know 
this myself until Sebastian educated me :)  Hence my strong preference 
on keeping this 'static' as an example for others who are as ignorant as 
I were ;) After all, having const data arrays static is quite an easy 
way of improving things - and it really does matter when there is many 
of arrays - or when they contain large data.

Yours
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~