Hi Linus & j, On Mon, May 29, 2017 at 6:42 PM, jmondi <jacopo@xxxxxxxxxx> 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 <jacopo@xxxxxxxxxx> 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 <linus.walleij@xxxxxxxxxx> 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 <linus.walleij@xxxxxxxxxx> 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
