gpio_of_helper and changing direction

GoogleGroups
1

I'm working on building a "universal" device tree overlay that can be
used to enable and switch between most of the various interesting pin
functions (uart, timers, spi, gpio, pru in/out, etc). One problem is
the BeagleBone specific gpio_of_helper driver designed to make it easy
to export GPIO pins via the device tree is forcing a fixed direction of
input or output on the GPIO pins.

Before I start hacking on the driver, I'm wondering why the pins weren't
either exported with the GPIOF_EXPORT_CHANGEABLE flag set, or why some
provision wasn't made to allow specifying the direction could be changed.

2

I verified the attached minor patch does what I need and allows the user
to change the GPIO direction after the pin is exported by the
gpio-of-helper driver.

Without this patch, any pins exported to sysfs by the gpio-of-helper
driver are missing the /sys/class/gpio/gpio<N>/direction file and are
stuck in either input or output mode.

Comments?

0003-Allow-direction-change-on-gpio-of-helper-exported-pi.patch (1.3 KB)

3

Great! It looks so obvious, this makes me wonder why it wasn't implemented the first time...!

-- Bas

4

Yes, but it's so simple I fear I'm missing something. There are
definitely times when you might not want an I/O pin to change direction,
and other times (like my use) where it's OK.

I think maybe I should add a new property to allow the direction to be
changed, so DT stubs would look something like:

x_min {
    gpio-name = "bebopr:x_min";
    gpio = <&gpio3 3 0>;
    input;
    dir-changeable;
};

x_ena {
    gpio-name = "bebopr:x_ena";
    gpio = <&gpio1 4 0>;
    output;
    init-high;
    dir-changeable;
};

Instead of just assuming the direction should _always_ be changeable, so
there's no chance of braking any existing setups.

5

Yes, I like this version much better. Patch attached.

Comments?

0003-Add-dir-changeable-property-to-gpio-of-helper.patch (868 Bytes)

6

Without this patch, any pins exported to sysfs by the gpio-of-helper
driver are missing the /sys/class/gpio/gpio<N>/direction file and are
stuck in either input or output mode.

Comments?

Great! It looks so obvious, this makes me wonder why it wasn't
implemented the first time...!

Yes, but it's so simple I fear I'm missing something. There are

That's what I meant to say. But since modern programmers do not write comments, we'll always keep wondering whether it was a design decision or an oversight. :-/

definitely times when you might not want an I/O pin to change direction,
and other times (like my use) where it's OK.

I think maybe I should add a new property to allow the direction to be
changed, so DT stubs would look something like:

x_min {
     gpio-name = "bebopr:x_min";
     gpio = <&gpio3 3 0>;
     input;
     dir-changeable;
};

I have limited knowledge of what you're trying to do, but you probably never want to make gpio3 an output on a BeBoPr because that creates two outputs fighting the signal!

x_ena {
     gpio-name = "bebopr:x_ena";
     gpio = <&gpio1 4 0>;
     output;
     init-high;
     dir-changeable;
};

Instead of just assuming the direction should _always_ be changeable, so
there's no chance of braking any existing setups.

Agreed, if possible, keep it backwards compatible.

-- Bas

7

Great! It looks so obvious, this makes me wonder why it wasn't
implemented the first time...!

Yes, but it's so simple I fear I'm missing something. There are

That's what I meant to say. But since modern programmers do not write
comments, we'll always keep wondering whether it was a design decision
or an oversight. :-/

Yep! <sigh>

definitely times when you might not want an I/O pin to change direction,
and other times (like my use) where it's OK.

I think maybe I should add a new property to allow the direction to be
changed, so DT stubs would look something like:

x_min {
     gpio-name = "bebopr:x_min";
     gpio = <&gpio3 3 0>;
     input;
     dir-changeable;
};

I have limited knowledge of what you're trying to do, but you probably
never want to make gpio3 an output on a BeBoPr because that creates two
outputs fighting the signal!

That's why I added the flag, so it can be left off of I/O pins that
really shouldn't be able to change direction. It still wound up being a
simple 2-line patch. I've tested the change works as expected and have
pushed the patch to my linux-dev github repo: