| Message ID | CACRpkdZySZqf-gPoQLGgMZ_B9m+8+Jbr4b6aA+j_OFDCHc2cGg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/10/2012 09:23 AM, Linus Walleij wrote: > On Sun, Sep 9, 2012 at 1:44 AM, Domenico Andreoli <cavokz@gmail.com> wrote: >> On Fri, Sep 07, 2012 at 11:57:59PM +0200, Linus Walleij wrote: >>> >>> 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 <swarren@wwwdotorg.org> 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: > >> 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
On 09/10/2012 01:34 PM, Linus Walleij wrote: > On Mon, Sep 10, 2012 at 7:41 PM, Stephen Warren <swarren@wwwdotorg.org> 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: >> >>> 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...
diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt index 1479aca..941e783 100644 --- a/Documentation/pinctrl.txt +++ b/Documentation/pinctrl.txt @@ -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 =================