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

From: Matti Vaittinen
Date: Sat Aug 20 2022 - 13:30:10 EST


On 8/20/22 19:09, Andy Shevchenko wrote:
On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
<mazziesaccount@xxxxxxxxx> wrote:
On 8/20/22 14:21, Jonathan Cameron wrote:
On Fri, 19 Aug 2022 22:19:17 +0300
Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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));