On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote:
On Fri, Sep 7, 2012 at 6:00 PM, Domenico Andreoli [off-list ref] wrote:
quoted
On Fri, Sep 07, 2012 at 02:30:59PM +0000, AnilKumar, Chimata wrote:
quoted
quoted
How can gpio driver knows that leds-gpio driver require
these 4 pins?
because leds-gpio requests each gpio (specified in the DT against a specific
gpio controller) before assuming it is available? gpiolib then asks to
pinctrl if those pins are available for gpio and possibly reserve them
(configuring the mux, if necessary) for the device.
So this is not an either-or situation but a both-and case.
...
If all you need to to is to multiplex the pins into GPIO mode,
then the gpio_get() call on this driver *can* call through to
pinctrl_request_gpio() which will in turn fall through to the
above pinmux driver calls (.gpio_request_enable, etc).
So if the GPIO driver doesn't coordinate with the pinctrl driver, it's
all left to the GPIO user to configure the pin before using it, right?
I can understand the concerns of Tony, whether a pin must be requested
or not before the gpio then depends on the GPIO driver implementation,
which may or may not call through the pinctrl layer, isn't it?.
But that's as far as it goes! The GPIO abstraction cannot
call through to e.g. set some specific biasing on the pins
(pull up etc). Doing that would require us to reimplement
every interface that pinctrl already has again in the
GPIO layer, which is not a good idea.
Yes, clear. Never meant that, I thought that the pinctrl was anyway
available for stuff not modeled by the GPIO layer, as you say below.
So the pinctrl handle can be used for such config, and it
can also be used for multiplexing if that is desired - if not
done by the fall through functions pinctrl_gpio_*().
You can use a combination of both too, Stephen patched
pinctrl some time back so that a pin can be muxed for a
certain function and used as GPIO at the same time, so
these two are now orthogonal, it's a bit relaxed and gives some
feeling of loss of control but was necessary for certain
usecases. (For example we can snoop on a I2C pin using
its corresponding GPIO registers in the U300...)
There is some flexibility here, I hope it's not too confusing :-/
On Sun, Sep 9, 2012 at 1:44 AM, Domenico Andreoli [off-list ref] wrote:
On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote:
quoted
If all you need to to is to multiplex the pins into GPIO mode,
then the gpio_get() call on this driver *can* call through to
pinctrl_request_gpio() which will in turn fall through to the
above pinmux driver calls (.gpio_request_enable, etc).
So if the GPIO driver doesn't coordinate with the pinctrl driver, it's
all left to the GPIO user to configure the pin before using it, right?
Yes, more or less, or should I say that certain aspects of pinctrl
are orthogonal to GPIO and the two mostly do not know of each
other due to a separation of concerns.
So the driver may need to tie things up and request its pinctrl
and GPIOs independently.
I can understand the concerns of Tony, whether a pin must be requested
or not before the gpio then depends on the GPIO driver implementation,
which may or may not call through the pinctrl layer, isn't it?.
Yes that is a sematic limitation, indeed.
I think the best way of trying to eliminate that is to bring the two
subsystems closer (which is some long-term project) but in the
meantime we could propose a documentation fixup to make the
semantics clear, what about this:
From a92d754367861cf564c09e0b15746e02f0a96f3f Mon Sep 17 00:00:00 2001
From: Linus Walleij <redacted>
Date: Mon, 10 Sep 2012 17:22:00 +0200
Subject: [PATCH] pinctrl: document semantics vs GPIO
The semantics of the interactions between GPIO and pinctrl may be
unclear, e.g. which one do you request first? This amends the
documentation to make this clear.
Reported-by: Domenico Andreoli <redacted>
Signed-off-by: Linus Walleij <redacted>
---
Documentation/pinctrl.txt | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
@@ -289,6 +289,29 @@ Interaction with the GPIO subsystem The GPIO drivers may want to perform operations of various types on the same physical pins that are also registered as pin controller pins.+First and foremost, the two subsystems can be used as completely orthogonal,+so say that your driver is fetching its resources like this:++#include <linux/pinctrl/consumer.h>+#include <linux/gpio.h>++struct pinctrl *pinctrl;+int gpio;++pinctrl = devm_pinctrl_get_select_default(&dev);+gpio = devm_gpio_request(&dev, 14, "foo");++Here we first request a certain pin state and then request GPIO 14 to be+used. If you're using the subsystems orthogonally like this, always get+your pinctrl handle and select the desired pinctrl state BEFORE requesting+the GPIO. This is a semantic convention to avoid situations that can be+electrically unpleasant, you may certainly want to mux in and bias pins+a certain way before the GPIO subsystems starts to deal with them.++But there are also situations where it makes sense for the GPIO subsystem+to communicate with with the pinctrl subsystem, using the latter as a+back-end.+ Since the pin controller subsystem have its pinspace local to the pin controller we need a mapping so that the pin control subsystem can figure out which pin controller handles control of a certain GPIO pin. Since a single
@@ -359,6 +382,7 @@ will get an pin number into its handled number
range. Further it is also passed
the range ID value, so that the pin controller knows which range it should
deal with.
+
PINMUX interfaces
=================
--
1.7.11.3
Yours,
Linus Walleij
From: Stephen Warren <hidden> Date: 2012-09-10 17:41:55
On 09/10/2012 09:23 AM, Linus Walleij wrote:
On Sun, Sep 9, 2012 at 1:44 AM, Domenico Andreoli [off-list ref] wrote:
quoted
On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote:
quoted
If all you need to to is to multiplex the pins into GPIO mode,
then the gpio_get() call on this driver *can* call through to
pinctrl_request_gpio() which will in turn fall through to the
above pinmux driver calls (.gpio_request_enable, etc).
So if the GPIO driver doesn't coordinate with the pinctrl driver, it's
all left to the GPIO user to configure the pin before using it, right?
Yes, more or less, or should I say that certain aspects of pinctrl
are orthogonal to GPIO and the two mostly do not know of each
other due to a separation of concerns.
So the driver may need to tie things up and request its pinctrl
and GPIOs independently.
That seems like exactly what we were trying to avoid when we added the
possibility for GPIO to call into pinctrl.
Documentation/gpio.txt already contains:
For GPIOs that use pins known to the pinctrl subsystem, that subsystem should
be informed of their use; a gpiolib driver's .request() operation may call
pinctrl_request_gpio(), and a gpiolib driver's .free() operation may call
pinctrl_free_gpio(). The pinctrl subsystem allows a pinctrl_request_gpio()
to succeed concurrently with a pin or pingroup being "owned" by a device for
pin multiplexing.
In order to resolve this, shouldn't we simply change the "should" at the
end of the first line I quoted to "must"? That way, there'd never be any
need to use pinctrl if you're only relying on gpiolib APIs.
(and I'd argue that the text was already meant to say "must", so this
isn't actually a change to the intent, just a clarification).
On Mon, Sep 10, 2012 at 7:41 PM, Stephen Warren [off-list ref] wrote:
On 09/10/2012 09:23 AM, Linus Walleij wrote:
That seems like exactly what we were trying to avoid when we added the
possibility for GPIO to call into pinctrl.
Documentation/gpio.txt already contains:
quoted
For GPIOs that use pins known to the pinctrl subsystem, that subsystem should
be informed of their use; a gpiolib driver's .request() operation may call
pinctrl_request_gpio(), and a gpiolib driver's .free() operation may call
pinctrl_free_gpio(). The pinctrl subsystem allows a pinctrl_request_gpio()
to succeed concurrently with a pin or pingroup being "owned" by a device for
pin multiplexing.
In order to resolve this, shouldn't we simply change the "should" at the
end of the first line I quoted to "must"? That way, there'd never be any
need to use pinctrl if you're only relying on gpiolib APIs.
(and I'd argue that the text was already meant to say "must", so this
isn't actually a change to the intent, just a clarification).
It should deal with all the simple muxing use cases yes. And
I am uncertain about the scope for this patch, if it only pertains
to muxing, and in that case it would be solved by adding
a proper GPIO backend to pinctrl-single.c.
But it doesn't help with some real-world usecases if I'm
not mistaken.
If you want to set up a certain GPIO pin as pull-down (I guess
this could be the case for a LED array), this cannot be done
through any of these functions:
extern int pinctrl_request_gpio(unsigned gpio);
extern void pinctrl_free_gpio(unsigned gpio);
extern int pinctrl_gpio_direction_input(unsigned gpio);
extern int pinctrl_gpio_direction_output(unsigned gpio);
So either we have to use a pin config hog to do this, or we have
to use devm_pinctrl_get_select_default(&pdev->dev); from the
driver (as this patch does). Either way it is using the pinctrl
system orthogonally to the GPIO system, it doesn't happen
from pinctrl_request_gpio() or so.
An alternative solution would be to add functions for
controlling pinconfig and whatnot to the GPIO glue, which
in turn would require adding frontends all over <linux/gpio.h>
which in turn was the thing that Grant nixed to I got
started with pinctrl instead...
But I'm open to any other suggestions. Would it be possible
for pinctrl_request_gpio() to activate a pin config in the
map for example? Currently it can only do muxing.
It's also possible to have the driver do something custom
behind the back of pinctrl altogether as a response to
pinctrl_request_gpio() but it wouldn't be
any elegant...
Yours,
Linus Walleij
From: Stephen Warren <hidden> Date: 2012-09-10 19:44:35
On 09/10/2012 01:34 PM, Linus Walleij wrote:
On Mon, Sep 10, 2012 at 7:41 PM, Stephen Warren [off-list ref] wrote:
quoted
On 09/10/2012 09:23 AM, Linus Walleij wrote:
quoted
That seems like exactly what we were trying to avoid when we added the
possibility for GPIO to call into pinctrl.
Documentation/gpio.txt already contains:
quoted
For GPIOs that use pins known to the pinctrl subsystem, that subsystem should
be informed of their use; a gpiolib driver's .request() operation may call
pinctrl_request_gpio(), and a gpiolib driver's .free() operation may call
pinctrl_free_gpio(). The pinctrl subsystem allows a pinctrl_request_gpio()
to succeed concurrently with a pin or pingroup being "owned" by a device for
pin multiplexing.
In order to resolve this, shouldn't we simply change the "should" at the
end of the first line I quoted to "must"? That way, there'd never be any
need to use pinctrl if you're only relying on gpiolib APIs.
(and I'd argue that the text was already meant to say "must", so this
isn't actually a change to the intent, just a clarification).
It should deal with all the simple muxing use cases yes. And
I am uncertain about the scope for this patch, if it only pertains
to muxing, and in that case it would be solved by adding
a proper GPIO backend to pinctrl-single.c.
But it doesn't help with some real-world usecases if I'm
not mistaken.
If you want to set up a certain GPIO pin as pull-down (I guess
this could be the case for a LED array), this cannot be done
through any of these functions:
extern int pinctrl_request_gpio(unsigned gpio);
extern void pinctrl_free_gpio(unsigned gpio);
extern int pinctrl_gpio_direction_input(unsigned gpio);
extern int pinctrl_gpio_direction_output(unsigned gpio);
So either we have to use a pin config hog to do this,
I'd certainly expect that to be the common case; I'd imagine it's pretty
common you'd never want to change the pulls at runtime, so hogging would
be appropriate.
or we have
to use devm_pinctrl_get_select_default(&pdev->dev); from the
driver (as this patch does).
Yes, true.
Either way it is using the pinctrl
system orthogonally to the GPIO system, it doesn't happen
from pinctrl_request_gpio() or so.
An alternative solution would be to add functions for
controlling pinconfig and whatnot to the GPIO glue, which
in turn would require adding frontends all over <linux/gpio.h>
which in turn was the thing that Grant nixed to I got
started with pinctrl instead...
Maybe the first gpio_request that GPIO passes to pinctrl could activate
some default "gpio" state or similar? But then you'd get into issues
with: what if the driver selects a pinctrl state for other reasons -
then you'd end up wanting multiple states active at once; the
gpiolib-requested state and the driver-requested state, and maybe they
conflict, ... probably madness ensues!
But I'm open to any other suggestions. Would it be possible
for pinctrl_request_gpio() to activate a pin config in the
map for example? Currently it can only do muxing.
It's also possible to have the driver do something custom
behind the back of pinctrl altogether as a response to
pinctrl_request_gpio() but it wouldn't be
any elegant...