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>
Cc: Matti Vaittinen <mazziesaccount@gmail.com>,
	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: Sat, 20 Aug 2022 06:48:40 +0000	[thread overview]
Message-ID: <0823a6e8-b325-78c5-d060-c5f9442e3df8@fi.rohmeurope.com> (raw)
In-Reply-To: <CAHp75VdGhXpD8YgwkVPLCBEMmupBiTDS4FChocJFVo+BBZ-2KA@mail.gmail.com>

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:
> 
> What did I miss?

 >>>>           struct bmg160_data *data;
 >>>>           struct iio_dev *indio_dev;

This does already violate the rule.

> 
>>> this case you even can move it out of the function, so we will see
>>> clearly that this is (not a hidden) global variable.
>>
>> Here I do disagree with you. Moving the array out of the function makes
>> it _much_ less obvious it is not used outside this function. Reason for
>> making is "static const" is to allow the data be placed in read-only
>> area (thanks to Guenter who originally gave me this tip).
> 
> "static" in C language means two things (that's what come to my mind):
> - for functions this tells that a function is not used outside of the module;
> - for variables that it is a _global_ variable.
> 
> Hiding static inside functions is not a good coding practice since it
> hides scope of the variable.

For const arrays the static in function does make sense. Being able to 
place the data in read-only areas do help with the memory on limited 
systems.

> And if you look into the kernel code, I
> believe the use you are proposing is in minority.

I don't know about the statistics. What I know is that we do have a 
technical benefits when we use static const arrays instead of non static 
ones in the functions. I do also believe placing the variables in blocks 
is a good practice.

I tend to agree with you that using local, non const statics has 
pitfalls - but the pitfalls do not really apply with const ones. You 
know the value and have no races. Benefit is that just by seeing that no 
pointer is returned you can be sure that no "sane code" uses the data 
outside the function it resides.

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