linux-kernel - Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170524203808.34aec03a@kernel.org>
Date:   Wed, 24 May 2017 20:38:08 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Vladimir Barinov <vladimir.barinov@...entembedded.com>
Cc:     Nikita Yushchenko <nikita.yoush@...entembedded.com>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        Matt Ranostay <mranostay@...il.com>,
        Gregor Boirie <gregor.boirie@...rot.com>,
        Sanchayan Maity <maitysanchayan@...il.com>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jeff White <Jeff.White@....aero>,
        Chris Healy <Chris.Healy@....aero>
Subject: Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio

On Wed, 24 May 2017 14:27:50 +0300
Vladimir Barinov <vladimir.barinov@...entembedded.com> wrote:

> On 23.05.2017 11:18, Nikita Yushchenko wrote:
> >>> Reset GPIO is active low.
> >>>
> >>> Currently driver uses gpiod_set_value(1) to clean reset, which depends
> >>> on device tree to contain GPIO_ACTIVE_HIGH - that does not match reality.
> >>>
> >>> This fixes driver to use _raw version of gpiod_set_value() to enforce
> >>> active-low semantics despite of what's written in device tree. Allowing
> >>> device tree to override that only opens possibility for errors and does
> >>> not add any value.
> >>>
> >>> Additionally, use _cansleep version to make things work with i2c-gpio
> >>> and other sleeping gpio drivers.  
> >> The reset gpio comes from platform hence it should be handled by DTS.
> >>
> >> In driver the gpio should not be raw.
> >>
> >> Even the hi8435 is active low but platform may invert signal (f.e. by
> >> adding trigger on the circuit path).  
> > I see.  However - isn't this pure theoretic?  Does such case exist?  
> I assure you that this is frequently used.
> 
> Simply search google for "simple voltage level shifter"
> It might be on PNP or NPN transistor, hence logic might be inverted.
> 
> >
> > In vast majority of cases, GPIO polarity is chip-specific, not
> > chip-use-specific.  Thus this knowlege belongs to driver and not to
> > device tree describing particular chip usage.  Having this always
> > defined at usage side is IMO major source of errors.  
> GPIO comes from SoC then "circuit path" and finally chip reset input.
> 
> What do you propose if h/w circuit path has simple voltage level shifter 
> on transistor. How to differentiate PNP and NPN cases?
> 
> Regards,
> Vladimir
> 

Hmm. Ah well, I clearly jumped too fast on this set and should have
left it for a while longer (I rushed a little as I'm away next weekend
and the cycle is moving towards rc3)

Sorry about that.

Anyhow, I am tempted to queue a revert of this patch.  The level
shifting case hadn't occurred to me (oops).

Thoughts?

I'm travelling at the end of this week, so may be the middle
of next before I can do anything about this one.

Jonathan