Accessing GPIOs from userspace using recent kernels

Accessing GPIOs from userspace using recent kernels

[Peter TB Brett]

Hi all,

We have several pieces of software that make use of the /sys/class/gpio 
userspace interface to the GPIO subsystem.

One important prerequisite to being able to use the userspace GPIO 
interface is to be able to put the corresponding package pins into GPIO 
mode, by modifying the chip's mux settings.

With Fedora 19 on the Pandaboard, we were able to achieve this by poking 
around in /sys/kernel/debug/omap_mux.  However, when running Fedora 20, 
this directory is empty.  My assumption is that this functionality has 
been subsumed by the pinctrl subsystem in recent kernels.  
Unfortunately, there doesn't seem to be any documented userspace API.

If there is no longer a userspace API for these settings, is there any 
way  that I can simply amend the device tree in order to change a couple 
of mux settings?

I've figured out that I would need a stanza that looks something like:

&omap4_pmx_core
{
  foo_pins: pinmux_foo_pins {
    pinctrl-single,pins = <
      0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* IRQ */
      0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* RST */
  };
};

But I can't work out what else I need to modify in order for these mux 
settings to take effect.

Thanks for any pointers!

                                                    Peter



-- 
Dr Peter Brett <peter@peter-b.co.uk>

Accessing GPIOs from userspace using recent kernels

[Javier Martinez Canillas]

Hello Peter,

On Wed, May 14, 2014 at 10:46 AM, Peter TB Brett <peter@peter-b.co.uk> wrote:
> Hi all,
>
> We have several pieces of software that make use of the /sys/class/gpio
> userspace interface to the GPIO subsystem.
>
> One important prerequisite to being able to use the userspace GPIO interface
> is to be able to put the corresponding package pins into GPIO mode, by
> modifying the chip's mux settings.
>
> With Fedora 19 on the Pandaboard, we were able to achieve this by poking
> around in /sys/kernel/debug/omap_mux.  However, when running Fedora 20, this
> directory is empty.  My assumption is that this functionality has been
> subsumed by the pinctrl subsystem in recent kernels.  Unfortunately, there
> doesn't seem to be any documented userspace API.
>

That is correct, OMAP now uses the pinctrl-single
(drivers/pinctrl/pinctrl-single.c) driver for pin configuration and
the old omap_mux debugfs interface no longer exists.

> If there is no longer a userspace API for these settings, is there any way
> that I can simply amend the device tree in order to change a couple of mux
> settings?
>

Not that I'm aware of, I've added Linus Walleij (the pinctrl subsystem
maintainer) on cc who will have an authoritative answer on this
subject.

> I've figured out that I would need a stanza that looks something like:
>
> &omap4_pmx_core
> {
>  foo_pins: pinmux_foo_pins {
>    pinctrl-single,pins = <
>      0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* IRQ */
>      0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* RST */
>  };
> };
>
> But I can't work out what else I need to modify in order for these mux
> settings to take effect.
>

You are only defining a set of pinmux configuration pins that will be
registered but that does not mean that the configuration will take
effect. Need to associate that configuration with a specific device
node so the state will be applicable. The good practice is to
associate with the device that needs that particular configuration or
if there is no such device then you can just add the properties in the
pinmux device node.

In your case would be something like should work:

&omap4_pmx_core
{
        pinctrl-names = "default";
        pinctrl-0 = <
                &foo_pins
        >;

        foo_pins: pinmux_foo_pins {
               pinctrl-single,pins = <
               0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* IRQ */
               0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* RST */
               >
        };
};

But as I said it would be better if you associate the pins with the
GPIO controller that contains those pins instead of the pinmux device.

> Thanks for any pointers!
>
>                                                    Peter

Hope it helps,

Javier

Accessing GPIOs from userspace using recent kernels

[Peter TB Brett]

Hi Javier,

Thank you for your helpful suggestions.

On 2014-05-14 11:00, Javier Martinez Canillas wrote:
> [snip]
> 
>> If there is no longer a userspace API for these settings, is there any 
>> way
>> that I can simply amend the device tree in order to change a couple of 
>> mux
>> settings?
>> 
> 
> Not that I'm aware of, I've added Linus Walleij (the pinctrl subsystem
> maintainer) on cc who will have an authoritative answer on this
> subject.

Having some sort of debugfs API for runtime modification of pinmux 
settings from userspace would be very helpful for hardware/firmware 
prototyping purposes.

> [snip]
> 
> You are only defining a set of pinmux configuration pins that will be
> registered but that does not mean that the configuration will take
> effect. Need to associate that configuration with a specific device
> node so the state will be applicable. The good practice is to
> associate with the device that needs that particular configuration or
> if there is no such device then you can just add the properties in the
> pinmux device node.
> 
> In your case would be something like should work:
> 
> &omap4_pmx_core
> {
>         pinctrl-names = "default";
>         pinctrl-0 = <
>                 &foo_pins
>         >;
> 
>         foo_pins: pinmux_foo_pins {
>                pinctrl-single,pins = <
>                0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* 
> IRQ */
>                0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* 
> RST */
>                >
>         };
> };

Note that this doesn't work; the omap4_pmx_core node already has a 
pinctrl-0 attribute with a whole bunch of entries (e.g. I2C pins and 
pins used for other peripherals) and there doesn't appear to be a way to 
append to the array rather than replacing it.

> But as I said it would be better if you associate the pins with the
> GPIO controller that contains those pins instead of the pinmux device.

Following this advice, I used:

&gpio1
{
   pinctrl-names = "default";
   pinctrl-0 = < &foo_pins >;
};

However, with these settings, nothing referring to pinmux_foo_pins 
appears in pinctrl-devices, pinctrl-maps, pinctrl-handles, or 
pinmux-pins, so I'm fairly sure these aren't the settings I'm looking 
for.

Is there any documentation for the debugfs interface to pinctrl?

                                                       Peter


-- 
Dr Peter Brett <peter@peter-b.co.uk>

Accessing GPIOs from userspace using recent kernels

[Javier Martinez Canillas]

Hello Peter,

On Thu, May 15, 2014 at 4:06 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
> Hi Javier,
>
> Thank you for your helpful suggestions.
>
> On 2014-05-14 11:00, Javier Martinez Canillas wrote:
>>
>> [snip]
>>
>>
>>> If there is no longer a userspace API for these settings, is there any
>>> way
>>> that I can simply amend the device tree in order to change a couple of
>>> mux
>>> settings?
>>>
>>
>> Not that I'm aware of, I've added Linus Walleij (the pinctrl subsystem
>> maintainer) on cc who will have an authoritative answer on this
>> subject.
>
>
> Having some sort of debugfs API for runtime modification of pinmux settings
> from userspace would be very helpful for hardware/firmware prototyping
> purposes.
>
>> [snip]
>>
>>
>> You are only defining a set of pinmux configuration pins that will be
>> registered but that does not mean that the configuration will take
>> effect. Need to associate that configuration with a specific device
>> node so the state will be applicable. The good practice is to
>> associate with the device that needs that particular configuration or
>> if there is no such device then you can just add the properties in the
>> pinmux device node.
>>
>> In your case would be something like should work:
>>
>> &omap4_pmx_core
>> {
>>         pinctrl-names = "default";
>>         pinctrl-0 = <
>>                 &foo_pins
>>         >;
>>
>>         foo_pins: pinmux_foo_pins {
>>                pinctrl-single,pins = <
>>                0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /*
>> IRQ */
>>                0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /*
>> RST */
>>                >
>>         };
>> };
>
>
> Note that this doesn't work; the omap4_pmx_core node already has a pinctrl-0
> attribute with a whole bunch of entries (e.g. I2C pins and pins used for
> other peripherals) and there doesn't appear to be a way to append to the
> array rather than replacing it.
>

In that case you can add the pinctrl single pins setup in the
omap4-panda-common.dtsi file as another element in pinctrl-0.

But I would first look why associating the pins with gpio1 does not work.

>
>> But as I said it would be better if you associate the pins with the
>> GPIO controller that contains those pins instead of the pinmux device.
>
>
> Following this advice, I used:
>
> &gpio1
>
> {
>   pinctrl-names = "default";
>   pinctrl-0 = < &foo_pins >;
> };
>
> However, with these settings, nothing referring to pinmux_foo_pins appears
> in pinctrl-devices, pinctrl-maps, pinctrl-handles, or pinmux-pins, so I'm
> fairly sure these aren't the settings I'm looking for.
>

That should work afaict, did you see any errors on the kernel log? can
you share your complete boot log?

Something that looks strange to me is that you say to be using a
Pandaboard and in your pasted DTS fragment you had:

0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* IRQ */
0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* RST *

but grepping for those registers numbers in omap4-panda-common.dtsi I
see that they refers to different mux pins and are already configured
for other devices in different modes:

$ git grep "0x5e\|0x12e" arch/arm/boot/dts/omap4-panda-common.dtsi

0x5e (PIN_INPUT_PULLUP | MUX_MODE0)     /* hdmi_sda.hdmi_sda */
0x12e (PIN_OUTPUT | MUX_MODE5)  /* dispc2_data15 */

Also I see that i2c4_scl has another register number and by the way is
also configured in another mode by i2c4 device node.
0xee (PIN_INPUT_PULLUP | MUX_MODE0)     /* i2c4_scl */

Is what you shared your real change or just an example that does not
apply to the Pandaboard? Could you please share your complete DTS?

> Is there any documentation for the debugfs interface to pinctrl?
>

No that I'm aware of, you should take a look to
drivers/pinctrl/{core,pinconf,pinmux}.c to figure out what information
each file exports.

Best regards,
Javier

Accessing GPIOs from userspace using recent kernels

[Peter TB Brett]

On 2014-05-15 19:54, Javier Martinez Canillas wrote:

> [snip]
> 
> But I would first look why associating the pins with gpio1 does not 
> work.
> 
>> 
>>> But as I said it would be better if you associate the pins with the
>>> GPIO controller that contains those pins instead of the pinmux 
>>> device.
>> 
>> 
>> Following this advice, I used:
>> 
>> &gpio1
>> 
>> {
>>   pinctrl-names = "default";
>>   pinctrl-0 = < &foo_pins >;
>> };
>> 
>> However, with these settings, nothing referring to pinmux_foo_pins 
>> appears
>> in pinctrl-devices, pinctrl-maps, pinctrl-handles, or pinmux-pins, so 
>> I'm
>> fairly sure these aren't the settings I'm looking for.
>> 
> 
> That should work afaict, did you see any errors on the kernel log? can
> you share your complete boot log?

Ah yes, there were some relevant messages:

[    2.939453] pinctrl-single 4a100040.pinmux: pin 4a10009e.0 already 
requested by 4a100040.pinmux; cannot claim for 4a310000.gpio
[    2.951843] pinctrl-single 4a100040.pinmux: pin-47 (4a310000.gpio) 
status -22
[    2.955261] pinctrl-single 4a100040.pinmux: could not request pin 47 
on device pinctrl-single

I should have spotted those.

> Something that looks strange to me is that you say to be using a
> Pandaboard and in your pasted DTS fragment you had:
> 
> 0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* IRQ */
> 0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* RST *
> 
> but grepping for those registers numbers in omap4-panda-common.dtsi I
> see that they refers to different mux pins and are already configured
> for other devices in different modes:
> 
> $ git grep "0x5e\|0x12e" arch/arm/boot/dts/omap4-panda-common.dtsi
> 
> 0x5e (PIN_INPUT_PULLUP | MUX_MODE0)     /* hdmi_sda.hdmi_sda */
> 0x12e (PIN_OUTPUT | MUX_MODE5)  /* dispc2_data15 */

Yes, that's my mistake.  I took the pin control register addresses from 
the OMAP4 documentation and forgot that you have to subtract 0x40 to get 
the correct address for use in the device tree.

The correct snippet has 0x1e and 0xee.

> Also I see that i2c4_scl has another register number and by the way is
> also configured in another mode by i2c4 device node.
> 0xee (PIN_INPUT_PULLUP | MUX_MODE0)     /* i2c4_scl */

The I2C4 controller is exposed on the Pandaboard's expansion header, but 
I want to use the pins for something else.  It seems that I can just 
override the i2c4 device node's pinctrl-0 attribute to be empty and thus 
prevent it from claiming pins?

> Is what you shared your real change or just an example that does not
> apply to the Pandaboard? Could you please share your complete DTS?

The attached .dts file sort of works-ish.  It's an ugly hack, but I 
don't have the time to do any more investigation into this now, 
unfortunately.

I guess my main question is: if I use /sys/class/gpio/export to export a 
GPIO for userspace control, it would make sense for the kernel to try 
and ensure that the GPIO is actually connected to something!  Please 
consider this a feature request for OMAP4's GPIO and pinctrl drivers.

The current call chain seems to be: gpiod_export() --> gpiod_request() 
--> omap_gpio_request().  Looking at other GPIO drivers, it seems like 
omap_gpio_request() should eventually call pinctrl_request_gpio().  
Would be useful if someone who knows about OMAP4/gpio/pinctrl could take 
a look at this.

                                      Peter

-- 
Dr Peter Brett <peter@peter-b.co.uk>

Accessing GPIOs from userspace using recent kernels

[Javier Martinez Canillas]

Hello Peter,

On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
> On 2014-05-15 19:54, Javier Martinez Canillas wrote:
>
>> [snip]
>>
>> But I would first look why associating the pins with gpio1 does not work.
>>
>>>
>>>> But as I said it would be better if you associate the pins with the
>>>> GPIO controller that contains those pins instead of the pinmux device.
>>>
>>>
>>>
>>> Following this advice, I used:
>>>
>>> &gpio1
>>>
>>> {
>>>   pinctrl-names = "default";
>>>   pinctrl-0 = < &foo_pins >;
>>> };
>>>
>>> However, with these settings, nothing referring to pinmux_foo_pins
>>> appears
>>> in pinctrl-devices, pinctrl-maps, pinctrl-handles, or pinmux-pins, so I'm
>>> fairly sure these aren't the settings I'm looking for.
>>>
>>
>> That should work afaict, did you see any errors on the kernel log? can
>> you share your complete boot log?
>
>
> Ah yes, there were some relevant messages:
>
> [    2.939453] pinctrl-single 4a100040.pinmux: pin 4a10009e.0 already
> requested by 4a100040.pinmux; cannot claim for 4a310000.gpio
> [    2.951843] pinctrl-single 4a100040.pinmux: pin-47 (4a310000.gpio) status
> -22
> [    2.955261] pinctrl-single 4a100040.pinmux: could not request pin 47 on
> device pinctrl-single
>

Yeah, I thought you had something like that after looking that you
were defining the same padconf registers twice.

> I should have spotted those.
>
>
>> Something that looks strange to me is that you say to be using a
>> Pandaboard and in your pasted DTS fragment you had:
>>
>> 0x5e (PIN_INPUT | MUX_MODE3)  /* gpmc_ad15.gpio_39 */ /* IRQ */
>> 0x12e (PIN_INPUT | MUX_MODE3) /* i2c4_scl.gpio_132 */ /* RST *
>>
>> but grepping for those registers numbers in omap4-panda-common.dtsi I
>> see that they refers to different mux pins and are already configured
>> for other devices in different modes:
>>
>> $ git grep "0x5e\|0x12e" arch/arm/boot/dts/omap4-panda-common.dtsi
>>
>> 0x5e (PIN_INPUT_PULLUP | MUX_MODE0)     /* hdmi_sda.hdmi_sda */
>> 0x12e (PIN_OUTPUT | MUX_MODE5)  /* dispc2_data15 */
>
>
> Yes, that's my mistake.  I took the pin control register addresses from the
> OMAP4 documentation and forgot that you have to subtract 0x40 to get the
> correct address for use in the device tree.
>
> The correct snippet has 0x1e and 0xee.
>
>

Ok, now makes more sense.

>> Also I see that i2c4_scl has another register number and by the way is
>> also configured in another mode by i2c4 device node.
>> 0xee (PIN_INPUT_PULLUP | MUX_MODE0)     /* i2c4_scl */
>
>
> The I2C4 controller is exposed on the Pandaboard's expansion header, but I
> want to use the pins for something else.  It seems that I can just override
> the i2c4 device node's pinctrl-0 attribute to be empty and thus prevent it
> from claiming pins?
>
>

Yes, if you need those pins to be setup in a different mode then of
course those should not be set by other devices.

>> Is what you shared your real change or just an example that does not
>> apply to the Pandaboard? Could you please share your complete DTS?
>
>
> The attached .dts file sort of works-ish.  It's an ugly hack, but I don't
> have the time to do any more investigation into this now, unfortunately.
>

I didn't get your .dts, are you sure that it was added as an attachment?

> I guess my main question is: if I use /sys/class/gpio/export to export a
> GPIO for userspace control, it would make sense for the kernel to try and
> ensure that the GPIO is actually connected to something!  Please consider
> this a feature request for OMAP4's GPIO and pinctrl drivers.
>

I'm not sure I understand what you mean with ensuring that the GPIO is
connected to something. Associating a GPIO (e.g: using the "gpios"
property) with a device node does not mean that the pin will be
configured in GPIO mode.

For example if you have some GPIOs connected to LEDs, then on your DTS
you will have a device node for your leds (compatible = "gpio-leds")
and a sub node for each led which will use a "gpios" property to
specify the GPIO pin that is connected to each led but that does not
mean that the padconf pins will be muxed in GPIO mode. You also need
to define the pinmux configuration and associate with the device in
order to be properly configured. E.g:

       leds {
                pinctrl-names = "default";
                pinctrl-0 = <&leds_pins>;
                compatible = "gpio-leds";

                boot {
                         label = "omap3:green:boot";
                         gpios = <&gpio1 26 GPIO_ACTIVE_HIGH>;
                         default-state = "on";
                };

                user0 {
                         label = "omap3:red:user0";
                         gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;
                         default-state = "off";
                };

                user1 {
                         label = "omap3:red:user1";
                         gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>;
                         default-state = "off";
                };
        };

&omap3_pmx_core2 {
...
        leds_pins: pinmux_leds_pins {
                pinctrl-single,pins = <
                        OMAP3630_CORE2_IOPAD(0x25f4, PIN_OUTPUT |
MUX_MODE4) /* etk_d12.gpio_26 */
                        OMAP3630_CORE2_IOPAD(0x25f6, PIN_OUTPUT |
MUX_MODE4) /* etk_d13.gpio_27 */
                        OMAP3630_CORE2_IOPAD(0x25f8, PIN_OUTPUT |
MUX_MODE4) /* etk_d14.gpio_28 */
                >;
        };
...
};


> The current call chain seems to be: gpiod_export() --> gpiod_request() -->
> omap_gpio_request().  Looking at other GPIO drivers, it seems like
> omap_gpio_request() should eventually call pinctrl_request_gpio().  Would be
> useful if someone who knows about OMAP4/gpio/pinctrl could take a look at
> this.
>

Some platforms use the same driver to control both pin
configuration/muxing and GPIO chips but OMAP uses two different
drivers to drive each hw block: drivers/pinctrl/pinctrl-single.c and
drivers/gpio/gpio-omap.c respectively.

So I don't know if what you are proposing only applies in that case
too but I've to admit that I'm not familiar with the pinctrl subsystem
so I'll take a look and see if calling pinctrl_{request,free}_gpio()
is actually missing in the GPIO OMAP driver.

Thanks a lot for your suggestion.

>
>                                      Peter
>
> --

Best regards,
Javier

Accessing GPIOs from userspace using recent kernels

[Linus Walleij]

Hi Peter,

I think you have already understood from the rest of the conversation that pin
control configuration shall be done in the device tree and not from userspace,
which is a good start.

As shown by Javier many things people sometimes do in userspace should
rather be done in kernelspace, such as controlling LEDs and reading
pushbuttons etc.

On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
> On 2014-05-15 19:54, Javier Martinez Canillas wrote:

>> 0x5e (PIN_INPUT_PULLUP | MUX_MODE0)     /* hdmi_sda.hdmi_sda */
>> 0x12e (PIN_OUTPUT | MUX_MODE5)  /* dispc2_data15 */
>
> Yes, that's my mistake.  I took the pin control register addresses from the
> OMAP4 documentation and forgot that you have to subtract 0x40 to get the
> correct address for use in the device tree.
>
> The correct snippet has 0x1e and 0xee.

I find the pinctrl-single hexdigit syntax infinitely complex and confusing,
but it was chosen by its designers. Most drivers use strings to configure
muxing and biasing etc.

>> Is what you shared your real change or just an example that does not
>> apply to the Pandaboard? Could you please share your complete DTS?
>
>
> The attached .dts file sort of works-ish.  It's an ugly hack, but I don't
> have the time to do any more investigation into this now, unfortunately.
>
> I guess my main question is: if I use /sys/class/gpio/export to export a
> GPIO for userspace control,

Which you should avoid, if possible.

> it would make sense for the kernel to try and
> ensure that the GPIO is actually connected to something!

How should we do that? The physics of that request evades me.

The pin control subsystem will however
refuse to use the pin if it is used for something else.

> The current call chain seems to be: gpiod_export() --> gpiod_request() -->
> omap_gpio_request().  Looking at other GPIO drivers, it seems like
> omap_gpio_request() should eventually call pinctrl_request_gpio().  Would be
> useful if someone who knows about OMAP4/gpio/pinctrl could take a look at
> this.

That is Tony Lindgren and the linux-omap mailing list.

Yours,
Linus Walleij

Accessing GPIOs from userspace using recent kernels

[Tony Lindgren]

* Linus Walleij <linus.walleij@linaro.org> [140523 04:36]:
> On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
> > The current call chain seems to be: gpiod_export() --> gpiod_request() -->
> > omap_gpio_request().  Looking at other GPIO drivers, it seems like
> > omap_gpio_request() should eventually call pinctrl_request_gpio().  Would be
> > useful if someone who knows about OMAP4/gpio/pinctrl could take a look at
> > this.

If we do this, we also need a solution to prevent automatic remuxing
of GPIO pins from happending. For wake-up events, some drivers need
to remux a rx pin to GPIO input for the duration of idle, and then
back to device rx pin. This is needed on some other platforms too
AFAIK.

For the drivers needing GPIO wake-up events, request_gpio is done in
the driver after drivers/base/pinctrl.c has already muxed the device
pin to rx. At minimum we would get warnings about reserved pins if
we tried to automatically mux them to GPIO.

We may be able to use some GPIO specific property to prevent
automatic remuxing as we discussed in the #armlinux few days ago.

Related to automatic remuxing of GPIO pins, there are also other
needs for pinctrl and GPIO interaction. We need to remux GPIO output
pins to input + pull + safe_mode to prevent the GPIO pins losing
value briefly during off-idle. That's the gpio errata 1.158 at as
shown at least at [1]. Because the GPIO to pinctrl mapping is
sparse and SoC specific, there's currently now obvious way to do
that. And we would need few new GPIO functions to tell pinctrl
subsystem about the change.

Regards,

Tony


[1] https://www.gitorious.org/rowboat/kernel/commit/86b15f21298b749a9d8216ff1839d33ad542464e?format=patch

Accessing GPIOs from userspace using recent kernels

[Javier Martinez Canillas]

Hello Tony,

On Fri, May 23, 2014 at 5:11 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Linus Walleij <linus.walleij@linaro.org> [140523 04:36]:
>> On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
>> > The current call chain seems to be: gpiod_export() --> gpiod_request() -->
>> > omap_gpio_request().  Looking at other GPIO drivers, it seems like
>> > omap_gpio_request() should eventually call pinctrl_request_gpio().  Would be
>> > useful if someone who knows about OMAP4/gpio/pinctrl could take a look at
>> > this.

I looked briefly at adding pinctrl back-end commands to the OMAP GPIO
driver. That is to make all GPIO operations fall through to the
pinctrl-single driver as Linus suggested before. The changes in the
GPIO driver are quite trivial, here is a RFC patch [0]  that has only
build tested but I think is useful to at least discuss this.

Now, in order to make that patch to actually work someone has to
register the chip GPIO range to pin controller mapping with
gpiochip_add_pin_range() or something similar to make
pinctrl_get_device_gpio_range() to succeed. The pinctrl-single driver
has a "pinctrl-single,gpio-range" property that can be used to define
a GPIO range to be registered.

But the problem is as you said that since there are two different hw
blocks for pin muxing and GPIO control, the pins that can be
multiplexed as GPIO are scattered all over the padconf registers
address space. So there isn't an easy way to specify the mapping and
we will have to add an entry on "pinctrl-single,gpio-range" for every
single GPIO pin (that's from 128 to 256 entries depending on the SoC).

To make even more complicated, the padconf registers offset for GPIO
pins are SoC specific so we need a mapping for each SoC.

So I wonder if is worth to add all that information to the DTS files.
Athough on the other hand is nice to have a better coordination
between the GPIO and pinctrl drivers and as Tony said there are use
cases where this is needed to workaround some silicion erratas.

>
> If we do this, we also need a solution to prevent automatic remuxing
> of GPIO pins from happending. For wake-up events, some drivers need
> to remux a rx pin to GPIO input for the duration of idle, and then
> back to device rx pin. This is needed on some other platforms too
> AFAIK.
>
> For the drivers needing GPIO wake-up events, request_gpio is done in
> the driver after drivers/base/pinctrl.c has already muxed the device
> pin to rx. At minimum we would get warnings about reserved pins if
> we tried to automatically mux them to GPIO.
>
> We may be able to use some GPIO specific property to prevent
> automatic remuxing as we discussed in the #armlinux few days ago.
>

Yes, adding a GPIO specific property to prevent automatic remuxing
sounds sensible to me.

> Related to automatic remuxing of GPIO pins, there are also other
> needs for pinctrl and GPIO interaction. We need to remux GPIO output
> pins to input + pull + safe_mode to prevent the GPIO pins losing
> value briefly during off-idle. That's the gpio errata 1.158 at as
> shown at least at [1]. Because the GPIO to pinctrl mapping is
> sparse and SoC specific, there's currently now obvious way to do
> that. And we would need few new GPIO functions to tell pinctrl
> subsystem about the change.
>

I'm not that familiar with the pinctrl-single driver but can't that
errata be handled on pinctrl-single without the GPIO OMAP driver
intervention?

I mean if we already add the complete GPIO <--> pin mapping using
"pinctrl-single,gpio-range" then the pinctrl-single driver will know
what pins can be muxed as GPIO and which ones were set as output with
pinctrl_gpio_direction_output() and then can just mux these output
GPIO pins to input + pull + safe_mode during off-idle. Or am I
completely lost here? :-)

> Regards,
>
> Tony
>
>
> [1] https://www.gitorious.org/rowboat/kernel/commit/86b15f21298b749a9d8216ff1839d33ad542464e?format=patch

Best regards,
Javier

[0]
commit 96c886987219e37395a160f8bd0d228509c1d4f0
Author: Javier Martinez Canillas <javier@dowhile0.org>
Date:   Fri May 30 20:50:39 2014 +0200

    gpio: omap: Make GPIO operations fall through pinctrl-single

    On OMAP platforms, there are two diferent hardware blocks for
    I/O multiplexing / pad configuration and GPIO control. So two
    different drivers are used: pinctrl-single and gpio-omap.

    Since two separate drivers are used there is no coordination
    between these and a I/O pad is not configured as a GPIO when
    a GPIO pin is requested.

    This patch adds pinctrl back-end commands to the GPIO OMAP
    driver so the pinmux_ops functions in the pinctrl-single
    driver are called for each GPIO operation.

    Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 00f29aa..cee63c6 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -27,6 +27,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/gpio.h>
 #include <linux/bitops.h>
+#include <linux/pinctrl/consumer.h>
 #include <linux/platform_data/gpio-omap.h>

 #define OFF_MODE       1
@@ -664,6 +665,11 @@ static int omap_gpio_request(struct gpio_chip
*chip, unsigned offset)
 {
        struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
        unsigned long flags;
+       int ret;
+
+       ret = pinctrl_request_gpio(chip->base + offset);
+       if (ret)
+               return ret;

        /*
         * If this is the first gpio_request for the bank,
@@ -704,6 +710,8 @@ static void omap_gpio_free(struct gpio_chip *chip,
unsigned offset)
         */
        if (!BANK_USED(bank))
                pm_runtime_put(bank->dev);
+
+       pinctrl_free_gpio(chip->base + offset);
 }

 /*
@@ -947,6 +955,11 @@ static int gpio_input(struct gpio_chip *chip,
unsigned offset)
 {
        struct gpio_bank *bank;
        unsigned long flags;
+       int ret;
+
+       ret = pinctrl_gpio_direction_input(chip->base + offset);
+       if (ret)
+               return ret;

        bank = container_of(chip, struct gpio_bank, chip);
        spin_lock_irqsave(&bank->lock, flags);
@@ -973,6 +986,11 @@ static int gpio_output(struct gpio_chip *chip,
unsigned offset, int value)
 {
        struct gpio_bank *bank;
        unsigned long flags;
+       int ret;
+
+       ret = pinctrl_gpio_direction_output(chip->base + offset);
+       if (ret)
+               return ret;

        bank = container_of(chip, struct gpio_bank, chip);
        spin_lock_irqsave(&bank->lock, flags);

Accessing GPIOs from userspace using recent kernels

[Tony Lindgren]

* Javier Martinez Canillas <javier@dowhile0.org> [140530 12:41]:
> Hello Tony,
> 
> On Fri, May 23, 2014 at 5:11 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Linus Walleij <linus.walleij@linaro.org> [140523 04:36]:
> >> On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
> >> > The current call chain seems to be: gpiod_export() --> gpiod_request() -->
> >> > omap_gpio_request().  Looking at other GPIO drivers, it seems like
> >> > omap_gpio_request() should eventually call pinctrl_request_gpio().  Would be
> >> > useful if someone who knows about OMAP4/gpio/pinctrl could take a look at
> >> > this.
> 
> I looked briefly at adding pinctrl back-end commands to the OMAP GPIO
> driver. That is to make all GPIO operations fall through to the
> pinctrl-single driver as Linus suggested before. The changes in the
> GPIO driver are quite trivial, here is a RFC patch [0]  that has only
> build tested but I think is useful to at least discuss this.
> 
> Now, in order to make that patch to actually work someone has to
> register the chip GPIO range to pin controller mapping with
> gpiochip_add_pin_range() or something similar to make
> pinctrl_get_device_gpio_range() to succeed. The pinctrl-single driver
> has a "pinctrl-single,gpio-range" property that can be used to define
> a GPIO range to be registered.
> 
> But the problem is as you said that since there are two different hw
> blocks for pin muxing and GPIO control, the pins that can be
> multiplexed as GPIO are scattered all over the padconf registers
> address space. So there isn't an easy way to specify the mapping and
> we will have to add an entry on "pinctrl-single,gpio-range" for every
> single GPIO pin (that's from 128 to 256 entries depending on the SoC).

Yes.
 
> To make even more complicated, the padconf registers offset for GPIO
> pins are SoC specific so we need a mapping for each SoC.
> 
> So I wonder if is worth to add all that information to the DTS files.
> Athough on the other hand is nice to have a better coordination
> between the GPIO and pinctrl drivers and as Tony said there are use
> cases where this is needed to workaround some silicion erratas.

I don't see any way around it short of adding the gpio to pinctrl
mappings for each SoC, and not do remuxing if the mapping is missing.

Then we need to make sure that the pinctrl register mappings are the
same for each SoC revision and package. If there are revision or package
specific changes, we'd have to handle those too somehow by specifying
the package with a compatible value in the board specific .dts file
or something like that.

> > If we do this, we also need a solution to prevent automatic remuxing
> > of GPIO pins from happending. For wake-up events, some drivers need
> > to remux a rx pin to GPIO input for the duration of idle, and then
> > back to device rx pin. This is needed on some other platforms too
> > AFAIK.
> >
> > For the drivers needing GPIO wake-up events, request_gpio is done in
> > the driver after drivers/base/pinctrl.c has already muxed the device
> > pin to rx. At minimum we would get warnings about reserved pins if
> > we tried to automatically mux them to GPIO.
> >
> > We may be able to use some GPIO specific property to prevent
> > automatic remuxing as we discussed in the #armlinux few days ago.
> >
> 
> Yes, adding a GPIO specific property to prevent automatic remuxing
> sounds sensible to me.

OK that probably needs to be discussed separately.
 
> > Related to automatic remuxing of GPIO pins, there are also other
> > needs for pinctrl and GPIO interaction. We need to remux GPIO output
> > pins to input + pull + safe_mode to prevent the GPIO pins losing
> > value briefly during off-idle. That's the gpio errata 1.158 at as
> > shown at least at [1]. Because the GPIO to pinctrl mapping is
> > sparse and SoC specific, there's currently now obvious way to do
> > that. And we would need few new GPIO functions to tell pinctrl
> > subsystem about the change.
> >
> 
> I'm not that familiar with the pinctrl-single driver but can't that
> errata be handled on pinctrl-single without the GPIO OMAP driver
> intervention?

No, it needs to happen from the GPIO driver based on runtime PM
calls. Only the GPIO driver knows when a GPIO output value is being
changed.
 
> I mean if we already add the complete GPIO <--> pin mapping using
> "pinctrl-single,gpio-range" then the pinctrl-single driver will know
> what pins can be muxed as GPIO and which ones were set as output with
> pinctrl_gpio_direction_output() and then can just mux these output
> GPIO pins to input + pull + safe_mode during off-idle. Or am I
> completely lost here? :-)

What about when a driver does gpio_set_value()? :)

> > [1] https://www.gitorious.org/rowboat/kernel/commit/86b15f21298b749a9d8216ff1839d33ad542464e?format=patch
> 
> [0]
> commit 96c886987219e37395a160f8bd0d228509c1d4f0
> Author: Javier Martinez Canillas <javier@dowhile0.org>
> Date:   Fri May 30 20:50:39 2014 +0200
> 
>     gpio: omap: Make GPIO operations fall through pinctrl-single
> 
>     On OMAP platforms, there are two diferent hardware blocks for
>     I/O multiplexing / pad configuration and GPIO control. So two
>     different drivers are used: pinctrl-single and gpio-omap.
> 
>     Since two separate drivers are used there is no coordination
>     between these and a I/O pad is not configured as a GPIO when
>     a GPIO pin is requested.
> 
>     This patch adds pinctrl back-end commands to the GPIO OMAP
>     driver so the pinmux_ops functions in the pinctrl-single
>     driver are called for each GPIO operation.
> 
>     Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>

This looks pretty straight forward, but we should not merge this
until the dependencies are sorted out first. And we should bail
out if no gpio to pinctrl mapping exists.

Regards,

Tony
 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 00f29aa..cee63c6 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -27,6 +27,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/gpio.h>
>  #include <linux/bitops.h>
> +#include <linux/pinctrl/consumer.h>
>  #include <linux/platform_data/gpio-omap.h>
> 
>  #define OFF_MODE       1
> @@ -664,6 +665,11 @@ static int omap_gpio_request(struct gpio_chip
> *chip, unsigned offset)
>  {
>         struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
>         unsigned long flags;
> +       int ret;
> +
> +       ret = pinctrl_request_gpio(chip->base + offset);
> +       if (ret)
> +               return ret;
> 
>         /*
>          * If this is the first gpio_request for the bank,
> @@ -704,6 +710,8 @@ static void omap_gpio_free(struct gpio_chip *chip,
> unsigned offset)
>          */
>         if (!BANK_USED(bank))
>                 pm_runtime_put(bank->dev);
> +
> +       pinctrl_free_gpio(chip->base + offset);
>  }
> 
>  /*
> @@ -947,6 +955,11 @@ static int gpio_input(struct gpio_chip *chip,
> unsigned offset)
>  {
>         struct gpio_bank *bank;
>         unsigned long flags;
> +       int ret;
> +
> +       ret = pinctrl_gpio_direction_input(chip->base + offset);
> +       if (ret)
> +               return ret;
> 
>         bank = container_of(chip, struct gpio_bank, chip);
>         spin_lock_irqsave(&bank->lock, flags);
> @@ -973,6 +986,11 @@ static int gpio_output(struct gpio_chip *chip,
> unsigned offset, int value)
>  {
>         struct gpio_bank *bank;
>         unsigned long flags;
> +       int ret;
> +
> +       ret = pinctrl_gpio_direction_output(chip->base + offset);
> +       if (ret)
> +               return ret;
> 
>         bank = container_of(chip, struct gpio_bank, chip);
>         spin_lock_irqsave(&bank->lock, flags);
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Accessing GPIOs from userspace using recent kernels

[Javier Martinez Canillas]

Hello Tony,

On Fri, May 30, 2014 at 10:25 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Javier Martinez Canillas <javier@dowhile0.org> [140530 12:41]:
>> Hello Tony,
>>
>> On Fri, May 23, 2014 at 5:11 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Linus Walleij <linus.walleij@linaro.org> [140523 04:36]:
>> >> On Fri, May 16, 2014 at 12:15 PM, Peter TB Brett <peter@peter-b.co.uk> wrote:
>> >> > The current call chain seems to be: gpiod_export() --> gpiod_request() -->
>> >> > omap_gpio_request().  Looking at other GPIO drivers, it seems like
>> >> > omap_gpio_request() should eventually call pinctrl_request_gpio().  Would be
>> >> > useful if someone who knows about OMAP4/gpio/pinctrl could take a look at
>> >> > this.
>>
>> I looked briefly at adding pinctrl back-end commands to the OMAP GPIO
>> driver. That is to make all GPIO operations fall through to the
>> pinctrl-single driver as Linus suggested before. The changes in the
>> GPIO driver are quite trivial, here is a RFC patch [0]  that has only
>> build tested but I think is useful to at least discuss this.
>>
>> Now, in order to make that patch to actually work someone has to
>> register the chip GPIO range to pin controller mapping with
>> gpiochip_add_pin_range() or something similar to make
>> pinctrl_get_device_gpio_range() to succeed. The pinctrl-single driver
>> has a "pinctrl-single,gpio-range" property that can be used to define
>> a GPIO range to be registered.
>>
>> But the problem is as you said that since there are two different hw
>> blocks for pin muxing and GPIO control, the pins that can be
>> multiplexed as GPIO are scattered all over the padconf registers
>> address space. So there isn't an easy way to specify the mapping and
>> we will have to add an entry on "pinctrl-single,gpio-range" for every
>> single GPIO pin (that's from 128 to 256 entries depending on the SoC).
>
> Yes.
>
>> To make even more complicated, the padconf registers offset for GPIO
>> pins are SoC specific so we need a mapping for each SoC.
>>
>> So I wonder if is worth to add all that information to the DTS files.
>> Athough on the other hand is nice to have a better coordination
>> between the GPIO and pinctrl drivers and as Tony said there are use
>> cases where this is needed to workaround some silicion erratas.
>
> I don't see any way around it short of adding the gpio to pinctrl
> mappings for each SoC, and not do remuxing if the mapping is missing.
>
> Then we need to make sure that the pinctrl register mappings are the
> same for each SoC revision and package. If there are revision or package
> specific changes, we'd have to handle those too somehow by specifying
> the package with a compatible value in the board specific .dts file
> or something like that.
>

Agreed.

>> > If we do this, we also need a solution to prevent automatic remuxing
>> > of GPIO pins from happending. For wake-up events, some drivers need
>> > to remux a rx pin to GPIO input for the duration of idle, and then
>> > back to device rx pin. This is needed on some other platforms too
>> > AFAIK.
>> >
>> > For the drivers needing GPIO wake-up events, request_gpio is done in
>> > the driver after drivers/base/pinctrl.c has already muxed the device
>> > pin to rx. At minimum we would get warnings about reserved pins if
>> > we tried to automatically mux them to GPIO.
>> >
>> > We may be able to use some GPIO specific property to prevent
>> > automatic remuxing as we discussed in the #armlinux few days ago.
>> >
>>
>> Yes, adding a GPIO specific property to prevent automatic remuxing
>> sounds sensible to me.
>
> OK that probably needs to be discussed separately.
>

Ok.

>> > Related to automatic remuxing of GPIO pins, there are also other
>> > needs for pinctrl and GPIO interaction. We need to remux GPIO output
>> > pins to input + pull + safe_mode to prevent the GPIO pins losing
>> > value briefly during off-idle. That's the gpio errata 1.158 at as
>> > shown at least at [1]. Because the GPIO to pinctrl mapping is
>> > sparse and SoC specific, there's currently now obvious way to do
>> > that. And we would need few new GPIO functions to tell pinctrl
>> > subsystem about the change.
>> >
>>
>> I'm not that familiar with the pinctrl-single driver but can't that
>> errata be handled on pinctrl-single without the GPIO OMAP driver
>> intervention?
>
> No, it needs to happen from the GPIO driver based on runtime PM
> calls. Only the GPIO driver knows when a GPIO output value is being
> changed.
>
>> I mean if we already add the complete GPIO <--> pin mapping using
>> "pinctrl-single,gpio-range" then the pinctrl-single driver will know
>> what pins can be muxed as GPIO and which ones were set as output with
>> pinctrl_gpio_direction_output() and then can just mux these output
>> GPIO pins to input + pull + safe_mode during off-idle. Or am I
>> completely lost here? :-)
>
> What about when a driver does gpio_set_value()? :)
>

Right, I didn't thought about that...

>> > [1] https://www.gitorious.org/rowboat/kernel/commit/86b15f21298b749a9d8216ff1839d33ad542464e?format=patch
>>
>> [0]
>> commit 96c886987219e37395a160f8bd0d228509c1d4f0
>> Author: Javier Martinez Canillas <javier@dowhile0.org>
>> Date:   Fri May 30 20:50:39 2014 +0200
>>
>>     gpio: omap: Make GPIO operations fall through pinctrl-single
>>
>>     On OMAP platforms, there are two diferent hardware blocks for
>>     I/O multiplexing / pad configuration and GPIO control. So two
>>     different drivers are used: pinctrl-single and gpio-omap.
>>
>>     Since two separate drivers are used there is no coordination
>>     between these and a I/O pad is not configured as a GPIO when
>>     a GPIO pin is requested.
>>
>>     This patch adds pinctrl back-end commands to the GPIO OMAP
>>     driver so the pinmux_ops functions in the pinctrl-single
>>     driver are called for each GPIO operation.
>>
>>     Signed-off-by: Javier Martinez Canillas <javier@dowhile0.org>
>
> This looks pretty straight forward, but we should not merge this
> until the dependencies are sorted out first. And we should bail
> out if no gpio to pinctrl mapping exists.
>

I agree, in fact that's why I didn't post as a proper patch but just
shared here as an RFC to show that the actual change in the GPIO OMAP
driver is quite small but the complete solution is not super trivial
due the other aspects we discussed above.

> Regards,
>
> Tony
>

Best regards,
Javier

>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 00f29aa..cee63c6 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -27,6 +27,7 @@
>>  #include <linux/irqchip/chained_irq.h>
>>  #include <linux/gpio.h>
>>  #include <linux/bitops.h>
>> +#include <linux/pinctrl/consumer.h>
>>  #include <linux/platform_data/gpio-omap.h>
>>
>>  #define OFF_MODE       1
>> @@ -664,6 +665,11 @@ static int omap_gpio_request(struct gpio_chip
>> *chip, unsigned offset)
>>  {
>>         struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
>>         unsigned long flags;
>> +       int ret;
>> +
>> +       ret = pinctrl_request_gpio(chip->base + offset);
>> +       if (ret)
>> +               return ret;
>>
>>         /*
>>          * If this is the first gpio_request for the bank,
>> @@ -704,6 +710,8 @@ static void omap_gpio_free(struct gpio_chip *chip,
>> unsigned offset)
>>          */
>>         if (!BANK_USED(bank))
>>                 pm_runtime_put(bank->dev);
>> +
>> +       pinctrl_free_gpio(chip->base + offset);
>>  }
>>
>>  /*
>> @@ -947,6 +955,11 @@ static int gpio_input(struct gpio_chip *chip,
>> unsigned offset)
>>  {
>>         struct gpio_bank *bank;
>>         unsigned long flags;
>> +       int ret;
>> +
>> +       ret = pinctrl_gpio_direction_input(chip->base + offset);
>> +       if (ret)
>> +               return ret;
>>
>>         bank = container_of(chip, struct gpio_bank, chip);
>>         spin_lock_irqsave(&bank->lock, flags);
>> @@ -973,6 +986,11 @@ static int gpio_output(struct gpio_chip *chip,
>> unsigned offset, int value)
>>  {
>>         struct gpio_bank *bank;
>>         unsigned long flags;
>> +       int ret;
>> +
>> +       ret = pinctrl_gpio_direction_output(chip->base + offset);
>> +       if (ret)
>> +               return ret;
>>
>>         bank = container_of(chip, struct gpio_bank, chip);
>>         spin_lock_irqsave(&bank->lock, flags);
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel