From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966064AbdEXTiN (ORCPT ); Wed, 24 May 2017 15:38:13 -0400 Received: from mail.kernel.org ([198.145.29.99]:39946 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964775AbdEXTiL (ORCPT ); Wed, 24 May 2017 15:38:11 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D4AA523990 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=jic23@kernel.org Date: Wed, 24 May 2017 20:38:08 +0100 From: Jonathan Cameron To: Vladimir Barinov Cc: Nikita Yushchenko , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Matt Ranostay , Gregor Boirie , Sanchayan Maity , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, Jeff White , Chris Healy Subject: Re: [PATCH 4/4] iio: hi8435: cleanup reset gpio Message-ID: <20170524203808.34aec03a@kernel.org> In-Reply-To: <5eec10c5-9e13-b0fc-fe40-a8b74a4fdbb0@cogentembedded.com> References: <20170519144802.14427-1-nikita.yoush@cogentembedded.com> <20170519144802.14427-4-nikita.yoush@cogentembedded.com> <18fc343b-d7a4-c5b8-84e6-702af6e94e44@cogentembedded.com> <4ba5a0ce-d443-bdf0-bb23-9941b7db59a4@cogentembedded.com> <5eec10c5-9e13-b0fc-fe40-a8b74a4fdbb0@cogentembedded.com> X-Mailer: Claws Mail 3.15.0-dirty (GTK+ 2.24.31; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 24 May 2017 14:27:50 +0300 Vladimir Barinov 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