Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751594AbdFIH1C (ORCPT ); Fri, 9 Jun 2017 03:27:02 -0400 Received: from mail-qt0-f196.google.com ([209.85.216.196]:34674 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751512AbdFIH07 (ORCPT ); Fri, 9 Jun 2017 03:26:59 -0400 MIME-Version: 1.0 In-Reply-To: <20170529104229.GB21347@w540> References: <20170508160120.GB25206@w540> <20170508172516.GC25206@w540> <20170523183735.GC13664@w540> <20170529104229.GB21347@w540> From: Dong Aisheng Date: Fri, 9 Jun 2017 15:26:57 +0800 Message-ID: Subject: Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable To: jmondi Cc: Linus Walleij , Andy Shevchenko , Chris Brandt , Jacopo Mondi , Geert Uytterhoeven , Laurent Pinchart , Rob Herring , Mark Rutland , Russell King - ARM Linux , Linux-Renesas , "linux-gpio@vger.kernel.org" , devicetree , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3924 Lines: 132 Hi Linus & j, On Mon, May 29, 2017 at 6:42 PM, jmondi wrote: > Hi Linus, > > On Mon, May 29, 2017 at 10:45:44AM +0200, Linus Walleij wrote: >> On Tue, May 23, 2017 at 8:37 PM, jmondi wrote: >> >> >> I did not follow too much. >> >> But it seems IMX7ULP/Vybrid to be also a fan of generic >> >> output-enable/input-enable >> >> property. >> >> >> >> See: >> >> Figure 5-2. GPIO PAD in Page 241 >> >> http://www.nxp.com/assets/documents/data/en/reference-manuals/VFXXXRM.pdf >> >> >> >> It has separate register bits to control input buffer enable and >> >> output buffer enable >> >> and we need set it property for GPIO function. >> > >> > As it seems we have another user for 'output-enable' here, what if we just >> > add that one to the generic bindings properties list, and we keep >> > 'bi-directional' (which seems to be the most debated property we have >> > added) out of generic properties? >> > >> > We can handle 'bi-directional' pins with static tables in our pin >> > controller driver and not have it anywhere in DT. >> >> This sounds like a viable approach. >> >> I just want to know if "output-enable" is the right name? >> "output-buffer-enable"? > > Great! Thanks! > > On naming: if we need "output-buffer-enable" should we add > "input-buffer-enable" as well? > > Currently we are using "input-enable" to pair with "output-enable", > but as you said, just "output-enable" when "output-high" and > "output-low" are there already seems a bit confusing. > At the same time "input-buffer-enable" seems to actually be just > electrically equivalent to "input-enable", so adding it is a bit of a > waste as well. > > I see three options here: > > 1) Add "output-buffer-enable" and "input-buffer-enable" > we end up with > "output-high" > "output-low" > "input-enable" > "output-buffer-enable" > "input-buffer-enable" > > 2) Add "output-buffer-enable" only > we end up with > "output-high" > "output-low" > "input-enable" > "output-buffer-enable" > > Binding may be confusing as in one case we use "output-buffer-enable" > while in the other "input-enable" > > 3) Add "output-enable" only > "output-high" > "output-low" > "input-enable" > "output-enable" > > As you, I don't like "output-enable" that much but it pairs better with > "input-enable". > > I'll let you and DT people decide on this, as it's really an ABI definition > problem and you have better judgment there. > What's the final decision of this? I saw the following revert patch in pinctrl-next but did not see a successive patch to add output-enable back? IMX7ULP pinctrl driver is pending on this because it needs use both input-enable and output-enable if we want to make them generic property. commit b4d2ea2af95cb77e2f320e24da526280d4aa2f6b Author: Linus Walleij Date: Mon May 8 10:48:21 2017 +0200 Revert "pinctrl: generic: Add bi-directional and output-enable" This reverts commit 8c58f1a7a4b6d1d723bf25fef9d842d5a11200d0. It turns out that applying these generic properties was premature: the properties used in the driver using this are of unclear electrical nature and the subject need to be discussed. Signed-off-by: Linus Walleij Regards Dong Aisheng >> >> > I see commit 42d5a11200d0[1] has not been reverted yet as Andy asked >> > in some previous email. >> >> I'm just overloaded. I sent that revert to Torvalds today. > > Thank you. Didn't want to put pressure ;) >> >> > I can send another version of that patch with >> > only 'output-enable' if you wish. >> >> That's what we want. >> >> > Once we reach consesus, I can then send v6 of our pin controller driver >> > based on that. >> >> OK sounds like a plan. >> >> Sorry for the mess, I'm just trying to get this right :/ > > Not a mess, and thanks for your effort in maintaining all of this > > Thanks > j >> >> Yours, >> Linus Walleij