On Friday, March 24, 2017 07:19:34 PM Krzysztof Kozlowski wrote:

On Fri, Mar 24, 2017 at 05:11:25PM +0100, Bartlomiej Zolnierkiewicz wrote:

quoted

On Friday, March 24, 2017 06:46:00 PM Krzysztof Kozlowski wrote:

quoted

I really do not like global or file-scope variables. I do not like
drivers using them. Actually I hate them.

From time to time I encounter a driver which was designed with that
approach - static fields and hidden assumption that there will be only
one instance. Usually that assumption is really hidden...

... and then it happens that I want to use two instances which of course
fails.

This code serves as a clear documentation for this assumption - only one
instance is allowed. You can look at it as a self-documenting
requirement.

For me it looks as needless case of defensive programming and when
I see the code like this it always raises questions about the real
intentions of the code. I find it puzzling and not helpful.

I do not understand what might be puzzling about check for static
file-scope value. It is of course subjective, but for me that looks
pretty self-explanatory.

The check should never happen given that ->probe will not happen twice.

However it seems that this is possible now with DT platform devices and
incorrect DTB.

quoted

quoted

And I think the probe might be called twice, for example in case of
mistake in DTB.

Even if this is possible resource allocation code in the driver will
take take care of handling it just fine,

Indeed, the devm_ioremap_resource() solves the case. I can drop the
check then.

Looking on this a bit more it seems that devm_ioremap_resource() will
not cover all mistakes (using compatible by mistake in some other DTB
node).

Leave the check, I take my objection back.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics