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

From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Miaoqian Lin <linmq006@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Xiang wangx <wangxiang@cdjrlc.com>,
	linux-iio <linux-iio@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 08/14] iio: bmg160_core: Simplify using devm_regulator_*get_enable()
Date: Mon, 22 Aug 2022 05:50:22 +0000	[thread overview]
Message-ID: <19d30866-1b2f-fd45-8ab7-3628319a70d2@fi.rohmeurope.com> (raw)
In-Reply-To: <CAHp75VfOYrDvNJ-ikGN2EU1NRjmQbiyfX-5TL2cUpZ1dreFBUQ@mail.gmail.com>

On 8/21/22 16:08, Andy Shevchenko wrote:
> On Sat, Aug 20, 2022 at 8:27 PM Matti Vaittinen
> <mazziesaccount@gmail.com> wrote:
>> On 8/20/22 19:21, Andy Shevchenko wrote:
>>> On Sat, Aug 20, 2022 at 1:05 PM Matti Vaittinen
>>> <mazziesaccount@gmail.com> wrote:
>>>> On 8/20/22 10:18, Andy Shevchenko wrote:
>>>>> On Sat, Aug 20, 2022 at 9:48 AM Vaittinen, Matti
>>>>> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
>>>>>> On 8/20/22 09:25, Andy Shevchenko wrote:
>>>>>>> On Sat, Aug 20, 2022 at 9:19 AM Vaittinen, Matti
>>>>>>> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
>>>>>>>> On 8/20/22 02:30, Andy Shevchenko wrote:
>>>>>>>>> On Fri, Aug 19, 2022 at 10:21 PM Matti Vaittinen
>>>>>>>>> <mazziesaccount@gmail.com> wrote:

//snip

> SInce it's static it's global by nature, but local by namespace.

Which is an _improvement_ over having it in global namespace?

>> It causes no more name collisions than a regular
>> local variable does so I really don't understand your reasoning.
> 
> And I have no other words to explain it to you. You are using a global
> variable in the scope of function. This is not good for the
> maintenance and development as it's prone to get an issue in the
> future.

If you foresee some issues, please describe them as I don't see one 
single problem with a local static const data. I have seen you telling 
me that "static const" variables are _harder_ for you to review. Could 
you please explain what are the potential mistake(s) a reviewer can do, 
and what is the 'issue' that mistake can cause?

>>> So, whom should we listen to here? Because bad code is bad code. And
>>> this is code above.
>>
>> Bad is a subjective concept. I'd say the code gets much worse if we make
>> the local variable a global one.
> 
> ...
> 
> 
> To summarize, we have a huge disagreement on the placement of the
> static variables. Not sure we ever get into compromize here, so I
> leave it up to maintainers, but my opinion that it is simply a bad
> code practice.

Bad and good are labels we can place on things. We however need to have 
the reason for those labels to be meaningful. I am sorry but I don't 
want to label the local _const_ static variables bad without reason. 
This discussion starts to remind me on statements like "using goto is 
always bad" or "one must never use macros in C".

Yeah - ultimately it is a maintainer decision.

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 ~~