Re: [PATCH] Input: gpio_keys: allocate pins

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 12, 2012 at 11:35 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Friday, October 12, 2012 11:26:00 PM Linus Walleij wrote:
>> On Fri, Oct 12, 2012 at 5:55 PM, Daniel Mack <zonque@xxxxxxxxx> wrote:
>> > This allows DT driven boards to allocate and configure the pinmux once
>> > the driver is probed.
>> >
>> > Signed-off-by: Daniel Mack <zonque@xxxxxxxxx>
>> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
>> > Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>>
>> (...)
>>
>> > +       /* request pin mux */

That is not good, pinctrl is not just about muxing.

>> > +       pinctrl = devm_pinctrl_get_select_default(dev);
>> > +       if (IS_ERR(pinctrl))
>> > +               dev_warn(dev, "pins are not configured from the
>> > driver\n");
>>
>> I think dev_warn() is rather nasty to throw in here, dev_info() is OK.
>>
>> However I suspect this driver could actually handle default, idle and sleep
>> states, especially after the runtime PM patches discussed elsewhere,
>> but that can be patched later.
>
> I take this as Acked-by then?

Not yet. I want to clarify the use. In the Documentation/pinctrl.txt
document we say:

-----------8<----------------------8<------------------8<-------------------
Drivers needing both pin control and GPIOs
==========================================

Again, it is discouraged to let drivers lookup and select pin control states
themselves, but again sometimes this is unavoidable.

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, you should
nominally 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 will certainly want to
mux in and bias pins in a certain way before the GPIO subsystems starts to
deal with them.

The above can be hidden: using pinctrl hogs, the pin control driver may be
setting up the config and muxing for the pins when it is probing,
nevertheless orthogonal to the GPIO subsystem.
But there are also situations where it makes sense for the GPIO subsystem
to communicate directly with with the pinctrl subsystem, using the latter
as a back-end. This is when the GPIO driver may call out to the functions
described in the section "Pin control interaction with the GPIO subsystem"
above. This only involves per-pin multiplexing, and will be completely
hidden behind the gpio_*() function namespace. In this case, the driver
need not interact with the pin control subsystem at all.

If a pin control driver and a GPIO driver is dealing with the same pins
and the use cases involve multiplexing, you MUST implement the pin controller
as a back-end for the GPIO driver like this, unless your hardware design
is such that the GPIO controller can override the pin controller's
multiplexing state through hardware without the need to interact with the
pin control system.
-----------8<----------------------8<------------------8<-------------------

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux