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 <[email protected]>
---
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)
--
2.37.1
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
On Fri, 19 Aug 2022 22:19:17 +0300
Matti Vaittinen <[email protected]> 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 <[email protected]>
>
> ---
> 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 <[email protected]> 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 <[email protected]>
>>
>> ---
>> 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! ~~
On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
<[email protected]> wrote:
> On 8/20/22 14:21, Jonathan Cameron wrote:
> > On Fri, 19 Aug 2022 22:19:17 +0300
> > Matti Vaittinen <[email protected]> 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.
--
With Best Regards,
Andy Shevchenko
On 8/20/22 19:09, Andy Shevchenko wrote:
> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
> <[email protected]> wrote:
>> On 8/20/22 14:21, Jonathan Cameron wrote:
>>> On Fri, 19 Aug 2022 22:19:17 +0300
>>> Matti Vaittinen <[email protected]> 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
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));
On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen
<[email protected]> wrote:
>
> On 8/20/22 19:09, Andy Shevchenko wrote:
> > On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
> > <[email protected]> wrote:
> >> On 8/20/22 14:21, Jonathan Cameron wrote:
> >>> On Fri, 19 Aug 2022 22:19:17 +0300
> >>> Matti Vaittinen <[email protected]> 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.
--
With Best Regards,
Andy Shevchenko
On 8/20/22 20:41, Andy Shevchenko wrote:
> On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen
> <[email protected]> wrote:
>>
>> On 8/20/22 19:09, Andy Shevchenko wrote:
>>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
>>> <[email protected]> wrote:
>>>> On 8/20/22 14:21, Jonathan Cameron wrote:
>>>>> On Fri, 19 Aug 2022 22:19:17 +0300
>>>>> Matti Vaittinen <[email protected]> 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
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));
On Sat, Aug 20, 2022 at 10:00 PM Matti Vaittinen
<[email protected]> wrote:
> On 8/20/22 20:41, Andy Shevchenko wrote:
> > On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen
> > <[email protected]> wrote:
> >> On 8/20/22 19:09, Andy Shevchenko wrote:
> >>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
> >>> <[email protected]> wrote:
> >>>> On 8/20/22 14:21, Jonathan Cameron wrote:
> >>>>> On Fri, 19 Aug 2022 22:19:17 +0300
> >>>>> Matti Vaittinen <[email protected]> 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.
--
With Best Regards,
Andy Shevchenko
On 8/21/22 16:13, Andy Shevchenko wrote:
> On Sat, Aug 20, 2022 at 10:00 PM Matti Vaittinen
> <[email protected]> wrote:
>> On 8/20/22 20:41, Andy Shevchenko wrote:
>>> On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen
>>> <[email protected]> wrote:
>>>> On 8/20/22 19:09, Andy Shevchenko wrote:
>>>>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
>>>>> <[email protected]> wrote:
>>>>>> On 8/20/22 14:21, Jonathan Cameron wrote:
>>>>>>> On Fri, 19 Aug 2022 22:19:17 +0300
>>>>>>> Matti Vaittinen <[email protected]> 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
--
The Linux Kernel guy at ROHM Semiconductors
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~ this year is the year of a signature writers block ~~
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 <[email protected]>
> Sent: Friday, August 19, 2022 9:19 PM
> To: Matti Vaittinen <[email protected]>; Matti Vaittinen
> <[email protected]>
> Cc: Sa, Nuno <[email protected]>; Jonathan Cameron
> <[email protected]>; Lars-Peter Clausen <[email protected]>; linux-
> [email protected]; [email protected]
> 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 <[email protected]>
>
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? <[email protected]>
On Fri, 19 Aug 2022 22:19:17 +0300
Matti Vaittinen <[email protected]> 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 <[email protected]>
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)