On 25 Sep 2014, stefan at agner.ch wrote:

Am 2014-09-25 11:07, schrieb Shawn Guo:

quoted

On Thu, Sep 25, 2014 at 09:00:41AM +0200, Stefan Agner wrote:

quoted

Am 2014-09-25 04:47, schrieb Shawn Guo:

quoted

On Tue, Sep 23, 2014 at 07:37:54PM +0200, Stefan Agner wrote:

quoted

quoted

quoted

quoted

Add pinmux support for GPIO for Vybrid (vf610) IOMUX controller.
This is needed since direction configuration is not part of the
GPIO module in Vybrid.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---

[snip]

quoted

quoted

quoted

quoted

+	reg = readl(ipctl->base + pin_reg->mux_reg);
+	reg &= ~(0x7 << 20);
+	writel(reg, ipctl->base + pin_reg->mux_reg);

quoted

quoted

quoted

Isn't this setup redundant at all, since imx_pmx_enable() already
takes care of setting mux register including GPIO mode?

quoted

quoted

Yes currently this is redundant, when a pinmux is actually
applied. What is the expected behaviour? Is a explicit pinmux
necessary before we can use GPIO? If not, maybe it would make more
sense to use imx_pmx_enable here to write all pinctrl settings?

quoted

Okay, as per Documentation/pinctrl.txt, it's required that GPIO and
PINCTRL can be used as orthogonal.  That said, your code does the
right thing.  Sorry for the noisy comment.

I'm happy you came up with this, since its the thing which I'm must
unsure whether this is right.

Right now, if you just export a random pin (which has no pinmux
configured in dt), you get an error since this function fails (mux_reg
== -1). But I guess we can't avoid it, we need the pinctrl framework
to have a valid setting before we fiddle around with the pad.

But now, if there is a pinmux configuration in dt, but its not applied
(using pinctrl-0 = ...), the export will succeed, however the GPIO
will not really be usable since no sane pad configuration is not
applied (input/output buffer disabled, no drive-strength)... So we
might well just don't change the mux here too.

IMHO, fully orthogonal is not possible, since on Vybrid those two
depend on each other. But in order to make it "more orthogonal", we
maybe should force applying the full configuration in
imx_pmx_gpio_request_enable (e.g. drive strenght etc), rather then
only mux the pad to to GPIO...

What do you think?

The muxing must have been done, that is correct.  However, this could be
by a boot loader, by some other API, etc.  People maybe concerned about
Linux affecting 'CAN' (or some 'mission critical' pins) muxing for
instance, but want the A5 to handle GPIO.

If somewhere, somehow the pin was muxed properly and the GPIO code still
works, I believe this is a win.  I see that this flexibility makes it
more difficult to get something that just works.  I think the device
trees take care of this for normal users.  It might be appropriate to
add a DT node that sets 'GPIO_CONTROL' and put a note in the DT-GPIO
document, that pinctrl is needed for a normal case or if some external
muxing is used or not needed, then 'control=false' can be set (or
whatever good DT terminology)?  Then the 'GPIO_CONTROL' check would be
in the 'vf610_gpio_direction_input()' functions.  The 'pinctrl'
functions are currently compiled to stubs if that is configured out.

Then there is 'multi-machine' support with DT and compile time selects
with CONFIG_PINCTRL and I don't think there is a lot of additional code
or confusion?

Fwiw,
Bill Pringlemeir.