Am 2014-09-25 18:43, schrieb Bill Pringlemeir:

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

quoted

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

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

quoted

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

quoted

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

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

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.

quoted

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

quoted

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.

quoted

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.

quoted

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...

quoted

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.

Currently pinctrl is not optional for Vybrid (Kconfig mandates pinctrl
by "select PINCTRL_VF610").

IMHO there is no real value having a way to mark a pin as "muxed
externally" (control=false). If we use a pin, we should configure and
mux that properly in Linux, and if we don't have valid configuration/mux
information, we should not touch that pin. 

In v2 of this patchset, the GPIO code only worked when there _is_ a
pinctrl entry for that GPIO pin in the DT. This is because I need the
register offset, which is provided by the entry itself. But the pad
configuration is another point, which I did not consider in v2. But in
v3, the GPIO code also writes the pad control.

So it was never possible to just use the GPIO driver for any pin, even
that pin was correctly muxed and configured externally. And, especially
on Vybrid, I think this is a good thing, since the pin could be used by
the M4 core. One has to have at least a proper entry in the device tree.
And in v3, I also check that this entry represents a valid GPIO mux
(VF610_PAD_*GPIO*).

IMHO with v3 we have a quite good and safe solution:
- Using pins as GPIO needs a valid DT entry
- Using that pin only requires exporting the pin, all
configuration/muxing get applied by imx_pmx_gpio_request_enable

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?

I don't understand that.

--
Stefan