On 09/10/2012 01:34 PM, Linus Walleij wrote: > On Mon, Sep 10, 2012 at 7:41 PM, Stephen Warren <swarren@xxxxxxxxxxxxx> 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... -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
