From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030819AbdEWSht (ORCPT ); Tue, 23 May 2017 14:37:49 -0400 Received: from relay4-d.mail.gandi.net ([217.70.183.196]:53238 "EHLO relay4-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S967922AbdEWSho (ORCPT ); Tue, 23 May 2017 14:37:44 -0400 Date: Tue, 23 May 2017 20:37:35 +0200 From: jmondi To: Dong Aisheng 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" Subject: Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable Message-ID: <20170523183735.GC13664@w540> References: <20170508160120.GB25206@w540> <20170508172516.GC25206@w540> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Linus, On Tue, May 23, 2017 at 06:08:31PM +0800, Dong Aisheng wrote: > On Tue, May 9, 2017 at 5:19 AM, Linus Walleij wrote: > > On Mon, May 8, 2017 at 7:25 PM, jmondi wrote: > > > >> From my perspective these flags are configurations internal to the pin > >> controller hardware used to enable/disable input buffers when a pin needs to > >> perform in both direction. > >> The level of detail I can provide on this is the logical diagram we have pointed > >> you to already. > >> > >> As I assume you are trying to get this answer from us in order to > >> avoid duplicating things in pin controller sub-system, and I > >> understand this, but my question here was "can we have those flags as part > >> of the pinmux property argument list, as that property description > >> seems to allow us to do that, instead of making them generic pin > >> configuration properties and upset other developers?" > > > > Pinmux with all it's magic flags baked into one is not any better > > or any more readable. The solution is already very pretty except > > for these two flags which I am sure we can agree on a way forward > > for. > > > > What we choose between is not this or another less transparent > > pin configuration mechanism, the mechanism (whether magic bits > > to pinmux or reasonable properties) does not matter. > > > > There is a strong preference to use the generic bindings. > > > > So the discussion is whether to use: > > > > bi-directional; > > output-enable; > > > > Or some already defined config flags. > > > > If these are too idiomatic to be used by others, they should anyways > > look similar, like: > > > > renesas,bi-directional; > > renesas,output-enable; > > > > Like the Qualcomm weirdness found in drivers/pinctrl/qcom/pinctrl-spmi-gpio.c > > > > qcom,pull-up-strength = <..>; > > > > Check how they use > > #define PMIC_GPIO_CONF_PULL_UP (PIN_CONFIG_END + 1) > > > > Etc. > > > >> Anyway, I still fail to see why those configuration flags, only > >> affecting the way the pin controller hardware enables/disables > >> its internal buffers and its internal operations have to be > >> described in term of their externally visible electrically characteristics. > > > > To me internal vs external is not what matters. What matters is > > if this is likely to pop up in more platforms, and then the property > > should be generic. > > > > The generic pin config definitions are likely to be picked up by other > > standards and even be inspiration to hardware engineers so that > > is why it matters so much. > > > >> To me, what already exists are pin configuration properties generic to > >> the whole pin controller subsystem, and I understand you don't want to > >> see duplication there. > >> > >> At the same time, to me, those flags are settings the pin controller > >> wants to have specified by software to overcome its hw design flaws, > >> and are intended to configure its internal buffers in a way it cannot > >> do by itself for some very specific operation modes (they are listed > >> in the hw reference manual, it's not something you can chose to > >> configure or not, if you want a pin working in i2c mode, you HAVE to > >> pass those flags to pin controller). > > > > Sounds like a case for > > > > renesas,bi-directional; > > renesas,output-enable; > > > > following the Qualcomm pattern in that case. > > > > But let's see if something else comes out of this discussion. > > > > 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. I see commit 42d5a11200d0[1] has not been reverted yet as Andy asked in some previous email. I can send another version of that patch with only 'output-enable' if you wish. Once we reach consesus, I can then send v6 of our pin controller driver based on that. Thanks j [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8c58f1a7a4b6d1d723bf25fef9d842d5a11200d0 > > Regards > Dong Aisheng