Thread (79 messages) 79 messages, 9 authors, 2017-06-19

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help