linux-kernel - Re: [RFD] Sharing GPIOs for input (buttons) and output (LEDs)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdb1J+-_9szuv6xWhrD7+NFeCjug+cqaYsHDYyLUYbLw1w@mail.gmail.com>
Date:	Thu, 17 Mar 2016 11:18:28 +0100
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	linux-renesas-soc@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFD] Sharing GPIOs for input (buttons) and output (LEDs)

On Mon, Feb 22, 2016 at 1:14 PM, Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:

> On the Renesas Salvator-X development board, 3 GPIO pins are connected to both
> push buttons and LEDs.
>   - If the GPIO is configured for output, it can control the LED,
>   - If the GPIO is configured for input, the push button status can be
>     read. Note that the LED is on if the push button is not pressed; it is
>     turned off while holding the button.

Have you asked the hardware engineer who did this construction
what s/he was thinking? And I mean seriously: what was the
usecase? Did they really design the LEDs to be used to flicker
with or just as an indication as to whether the button was being
pushed or not?

Your approach seems dedicated to use it for both usecases (also
as a stand-alone heartbeat or whatever) but was that really
intended?

>         keyboard {
>                 compatible = "gpio-keys";
>
>                 key-a {
>                         gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
>                         label = "SW20";
>                         wakeup-source;
>                         linux,code = <KEY_A>;
>                 };
>                 key-b {
>                         gpios = <&gpio6 12 GPIO_ACTIVE_LOW>;
>                         label = "SW21";
>                         wakeup-source;
>                         linux,code = <KEY_B>;
>                 };
>                 key-c {
>                         gpios = <&gpio6 13 GPIO_ACTIVE_LOW>;
>                         label = "SW22";
>                         wakeup-source;
>                         linux,code = <KEY_C>;
>                 };
>         };

I suspect that in this usecase, the GPIO should be flagged
GPIO_OPEN_DRAIN rather than GPIO_ACTIVE_LOW,
as the construction with the LED draws current fron the line.

> There exist device tree bindings for LEDs connected to GPIOs, and the
> following also works:
>
>         leds {
>                 compatible = "gpio-leds";
>
>                 led4 {
>                         gpios = <&gpio6 11 GPIO_ACTIVE_HIGH>;
>                         label = "LED4";
>                 };
>                 led5 {
>                         gpios = <&gpio6 12 GPIO_ACTIVE_HIGH>;
>                         label = "LED5";
>                 };
>                 led6 {
>                         gpios = <&gpio6 13 GPIO_ACTIVE_HIGH>;
>                         label = "LED6";
>                 };
>         };

OK

> If a GPIO is found busy during initialization, and if it's already in
> use by the other half of the driver, the driver switches to a special
> "polled-key-and-LED" mode. I.e. during polling, it does:
>   - Save the GPIO output state,
>   - Switch the GPIO to input mode,
>   - Wait 5 ms (else it will read the old output state, depending on
>     e.g. hardware capacitance),
>   - Read the GPIO input state,
>   - Switch the GPIO to output mode,
>   - Restore the GPIO output state.
>
> And it works, the LEDs can be controlled, and the push button states can
> be read!

Why go to such troubles of switching the line from output to
input to read it?

Especially when a line is OPEN_DRAIN it should be perfectly legal
to read it's value even if it is set as output, but AFAICT that is
always working no matter whether the line is set as output.

> However, due to the 5 ms delay, there's a visible flickering of LEDs
> that are supposed to be turned off (remember, when the GPIO is
> configured for input and the button is not pressed, the LED is lit).
>
> If we go this route, adding support for non-polled GPIOs (if the GPIO is
> not shared with an LED) and wake-up should be doable.

I think this actually implies OPEN_DRAIN and if you flag it as such
the core should be happy using it as input and output at the same
time.

If the hardware has a problem with reading the value from a line
that is set to output, it needs a workaround hack in the driver to
support reading and output line, *NOT* changes to gpiolib,
because the lib assumes this is always possible, i.e. it will
call the driver .get() callback no matter what.

Yours,
Linus Walleij