| Message ID | a29493f594c84b3bd852e462bbd3e591a8575a27.1660934107.git.mazziesaccount@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Use devm helpers for regulator get and enable | expand |
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)
On 8/20/22 14:21, Jonathan Cameron wrote: > 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. 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
On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen <mazziesaccount@gmail.com> wrote: > On 8/20/22 14:21, Jonathan Cameron wrote: > > On Fri, 19 Aug 2022 22:19:17 +0300 > > Matti Vaittinen <mazziesaccount@gmail.com> wrote: ... > >> + 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. And also make it a reversed xmas tree order. > > 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. But still the same comment about global scope of the variable is applied. As I explained before, hiding global variables inside a function is a bad code practice.
On 8/20/22 19:09, Andy Shevchenko wrote: > On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen > <mazziesaccount@gmail.com> wrote: >> On 8/20/22 14:21, Jonathan Cameron wrote: >>> On Fri, 19 Aug 2022 22:19:17 +0300 >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > ... > >>>> + 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. > > And also make it a reversed xmas tree order. can do. > >>> 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. > > But still the same comment about global scope of the variable is applied. I don't understand why you keep claiming the variable is global when it is not? > As I explained before, hiding global variables inside a function is a > bad code practice. I don't really get what you mean here. And I definitely don't see any improvement if we would really use a global variable instead of a local one. --Matti
On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > On 8/20/22 19:09, Andy Shevchenko wrote: > > On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen > > <mazziesaccount@gmail.com> wrote: > >> On 8/20/22 14:21, Jonathan Cameron wrote: > >>> On Fri, 19 Aug 2022 22:19:17 +0300 > >>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: ... > >>> 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. > > > > But still the same comment about global scope of the variable is applied. > > I don't understand why you keep claiming the variable is global when it > is not? It is. The static keyword makes it global, but putting the entire definition into the function is asking for troubles. I guess some C standard chapter describes that in non-understandable language. > > As I explained before, hiding global variables inside a function is a > > bad code practice. > > I don't really get what you mean here. And I definitely don't see any > improvement if we would really use a global variable instead of a local one. The improvement is avoid hiding the global variable to the local namespace.
On 8/20/22 20:41, Andy Shevchenko wrote: > On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen > <mazziesaccount@gmail.com> wrote: >> >> On 8/20/22 19:09, Andy Shevchenko wrote: >>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen >>> <mazziesaccount@gmail.com> wrote: >>>> On 8/20/22 14:21, Jonathan Cameron wrote: >>>>> On Fri, 19 Aug 2022 22:19:17 +0300 >>>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > ... > >>>>> 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. >>> >>> But still the same comment about global scope of the variable is applied. >> >> I don't understand why you keep claiming the variable is global when it >> is not? > > It is. The static keyword makes it global, but putting the entire > definition into the function is asking for troubles. > Please, describe the trouble we can get with a local static const variable? A real concrete threat there is. I have explained the benefit. I have also explained the concrete possibility of name collision when we really do a global out of local. > I guess some C standard chapter describes that in non-understandable language. > >>> As I explained before, hiding global variables inside a function is a >>> bad code practice. >> >> I don't really get what you mean here. And I definitely don't see any >> improvement if we would really use a global variable instead of a local one. > > The improvement is avoid hiding the global variable to the local namespace. I guess you mean that you may miss the fact that a variable stays there even after execution exits the function, right? Ok, let's assume someone misses this point when reading the code. Now, please describe me the potential issues this can cause knowing our static is const and doesn't change the value. Best Regards -- Matti
On Sat, Aug 20, 2022 at 10:00 PM Matti Vaittinen <mazziesaccount@gmail.com> wrote: > On 8/20/22 20:41, Andy Shevchenko wrote: > > On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen > > <mazziesaccount@gmail.com> wrote: > >> On 8/20/22 19:09, Andy Shevchenko wrote: > >>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen > >>> <mazziesaccount@gmail.com> wrote: > >>>> On 8/20/22 14:21, Jonathan Cameron wrote: > >>>>> On Fri, 19 Aug 2022 22:19:17 +0300 > >>>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: > > > > ... > > > >>>>> 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. > >>> > >>> But still the same comment about global scope of the variable is applied. > >> > >> I don't understand why you keep claiming the variable is global when it > >> is not? > > > > It is. The static keyword makes it global, but putting the entire > > definition into the function is asking for troubles. > Please, describe the trouble we can get with a local static const > variable? A real concrete threat there is. I have explained the benefit. > I have also explained the concrete possibility of name collision when we > really do a global out of local. I told you, the benefit is not to play dirty tricks on developers, maintainers and reviewers. It's simply harder to read the code and get the usage of the variable that lifetime is out of scope of the function. > > I guess some C standard chapter describes that in non-understandable language. > > > >>> As I explained before, hiding global variables inside a function is a > >>> bad code practice. > >> > >> I don't really get what you mean here. And I definitely don't see any > >> improvement if we would really use a global variable instead of a local one. > > > > The improvement is avoid hiding the global variable to the local namespace. > > I guess you mean that you may miss the fact that a variable stays there > even after execution exits the function, right? Ok, let's assume someone > misses this point when reading the code. Now, please describe me the > potential issues this can cause knowing our static is const and doesn't > change the value. When you hide the static variable inside the function, you simply narrow visibility to the compiler, but the variable stays all the time the module is in.
On 8/21/22 16:13, Andy Shevchenko wrote: > On Sat, Aug 20, 2022 at 10:00 PM Matti Vaittinen > <mazziesaccount@gmail.com> wrote: >> On 8/20/22 20:41, Andy Shevchenko wrote: >>> On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen >>> <mazziesaccount@gmail.com> wrote: >>>> On 8/20/22 19:09, Andy Shevchenko wrote: >>>>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen >>>>> <mazziesaccount@gmail.com> wrote: >>>>>> On 8/20/22 14:21, Jonathan Cameron wrote: >>>>>>> On Fri, 19 Aug 2022 22:19:17 +0300 >>>>>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote: >>> >>> ... >>> >>>>>>> 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. >>>>> >>>>> But still the same comment about global scope of the variable is applied. >>>> >>>> I don't understand why you keep claiming the variable is global when it >>>> is not? >>> >>> It is. The static keyword makes it global, but putting the entire >>> definition into the function is asking for troubles. > >> Please, describe the trouble we can get with a local static const >> variable? A real concrete threat there is. I have explained the benefit. >> I have also explained the concrete possibility of name collision when we >> really do a global out of local. > > I told you, the benefit is not to play dirty tricks on developers, > maintainers and reviewers. I see nothing concrete in that statement. > It's simply harder to read the code and get > the usage of the variable that lifetime is out of scope of the > function. This still makes no sense to me. Lifetime is the same even if we put the variable out of the function as you suggested. It does not change. And a reviewer missing that fact for a const data does really not matter. Variable is there when function is entered, it has always the same value - it just is not in the stack. I agree that a variable which value may change between function calls is more difficult to track when it is static. This is not the case here. For a reviewer or code reader it is actually _much simpler_ to see where the variable is used when we put it in the block where it is used. If I did as you suggested and pulled it out of the function then every code reader should grep the whole file to detect the use. Now readers only need to check if a pointer to the variable is returned out of the function. Oh, and this same check should be done for truly global variables too - as users can store the pointer to something which does not match the grep. So, again. Putting the variables (also static ones) in the blocks do make code reading and reviewing _easier_. > >>> I guess some C standard chapter describes that in non-understandable language. >>> >>>>> As I explained before, hiding global variables inside a function is a >>>>> bad code practice. >>>> >>>> I don't really get what you mean here. And I definitely don't see any >>>> improvement if we would really use a global variable instead of a local one. >>> >>> The improvement is avoid hiding the global variable to the local namespace. >> >> I guess you mean that you may miss the fact that a variable stays there >> even after execution exits the function, right? Ok, let's assume someone >> misses this point when reading the code. Now, please describe me the >> potential issues this can cause knowing our static is const and doesn't >> change the value. > > When you hide the static variable inside the function, you simply > narrow visibility to the compiler, but the variable stays all the time > the module is in. Yes. The constant, unchanging data stays there all the time. How does it make your reviewing harder? Why do you care whether the data stays in the same place or not? What you should be interested is where and how the data is accessed - and this is where having the variable local will actually help you. More I think of this, less I can see the problem you see :( Best Regards -- Matti
Interesting to see the different mental models used when reading code. Overall I'm fine with either static within function or static outside function depending on author preference + for short cases like this I'm also fine with them on the stack. Obviously can only speak for what I'll take into IIO! Jonathan
> From: Matti Vaittinen <mazziesaccount@gmail.com> > Sent: Friday, August 19, 2022 9:19 PM > To: Matti Vaittinen <mazziesaccount@gmail.com>; Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> > Cc: Sa, Nuno <Nuno.Sa@analog.com>; Jonathan Cameron > <jic23@kernel.org>; Lars-Peter Clausen <lars@metafoo.de>; linux- > iio@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH v3 07/14] iio: ltc2688: Simplify using > devm_regulator_*get_enable() > > [External] > > 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> > FWIW, I also don't care that much about the whole variable scope discussion... Hence, with the change requested by Jonathan: Reviewed-by: Nuno Sá <nuno.sa@analog.com>
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> Applied with the ordering tweaked as per Andy's review. Also tweaked the patch title to iio: dac: ltc2688: just to make it a bit easier to go from patch title to driver. > > --- > 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"}; > > 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)
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"}; 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)
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(-)