Re: [PATCH v5 01/10] pinctrl: generic: Add bi-directional and output-enable
From: jmondi <jacopo@jmondi.org>
Date: 2017-05-29 10:42:50
Also in:
linux-gpio, linux-renesas-soc, lkml
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 [off-list ref] wrote:quoted
quoted
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.
quoted
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 ;)
quoted
I can send another version of that patch with only 'output-enable' if you wish.That's what we want.quoted
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