Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver

From	Krzysztof Kozlowski <krzk@kernel.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver
Date	2017-03-24 17:30 +0100
Message-ID	<toAgb-4c4-33@gated-at.bofh.it> (permalink)
References	<toyo1-2Pm-3@gated-at.bofh.it> <tozk5-3uM-1@gated-at.bofh.it> <tozDs-3Gl-29@gated-at.bofh.it> <toA6t-48t-9@gated-at.bofh.it> <toA6t-48t-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Mar 24, 2017 at 05:11:25PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday, March 24, 2017 06:46:00 PM Krzysztof Kozlowski wrote:
> > 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.

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

Best regards,
Krzysztof

Thread

[PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 15:30 +0100
  Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Stephan Müller <smueller@chronox.de> - 2017-03-24 15:40 +0100
    Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Stephan Müller <smueller@chronox.de> - 2017-03-24 15:50 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 16:00 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Stephan Müller <smueller@chronox.de> - 2017-03-24 16:00 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 16:00 +0100
    Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 15:50 +0100
  Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2017-03-24 16:30 +0100
    Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 16:50 +0100
      Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2017-03-24 17:20 +0100
        Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 17:30 +0100
          Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2017-03-24 17:50 +0100
            Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Krzysztof Kozlowski <krzk@kernel.org> - 2017-03-24 18:10 +0100

csiph-web