[PATCH 0/2] Salvator-X: Add GPIO keys support

[PATCH 0/2] Salvator-X: Add GPIO keys support

[Laurent Pinchart]

Hello,

This simple series adds GPIO keys support to both the H3- and M3-W-based
Salvator-X boards. Please see individual patches for details.

Laurent Pinchart (2):
  arm64: dts: r8a7795: salvator-x: Add GPIO keys support
  arm64: dts: r8a7796: salvator-x: Add GPIO keys support

 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 63 ++++++++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 63 ++++++++++++++++++++++
 2 files changed, 126 insertions(+)

-- 
Regards,

Laurent Pinchart

[PATCH 1/2] arm64: dts: r8a7795: salvator-x: Add GPIO keys support

[Laurent Pinchart]

The Salvator-X board as a 4 lines DIP switch and 3 push buttons
connected to SoC GPIOs, meant to be used as general-purpose test keys.
Add a corresponding node in DT, mapping (semi-randomly) the DIP switch
to keys 1-4 and the push buttons to keys A-C.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 63 ++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
index bcaf4008d32d..ef3511cab367 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -34,6 +34,7 @@
 /dts-v1/;
 #include "r8a7795.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 
 / {
 	model = "Renesas Salvator-X board based on r8a7795";
@@ -56,6 +57,63 @@
 		reg = <0x0 0x48000000 0x0 0x38000000>;
 	};
 
+	keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&keys_pins>;
+		pinctrl-names = "default";
+
+		key-1 {
+			gpios = <&gpio5 17 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_1>;
+			label = "SW4-1";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-2 {
+			gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_2>;
+			label = "SW4-2";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-3 {
+			gpios = <&gpio5 22 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_3>;
+			label = "SW4-3";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-4 {
+			gpios = <&gpio5 23 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_4>;
+			label = "SW4-4";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-a {
+			gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_A>;
+			label = "TSW0";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-b {
+			gpios = <&gpio6 12 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_B>;
+			label = "TSW1";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-c {
+			gpios = <&gpio6 13 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_C>;
+			label = "TSW2";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+	};
+
 	x12_clk: x12_clk {
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
@@ -256,6 +314,11 @@
 		function = "du";
 	};
 
+	keys_pins: keys {
+		pins = "GP_5_17", "GP_5_20", "GP_5_22";
+		bias-pull-up;
+	};
+
 	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
-- 
Regards,

Laurent Pinchart

[PATCH 2/2] arm64: dts: r8a7796: salvator-x: Add GPIO keys support

[Laurent Pinchart]

The Salvator-X board as a 4 lines DIP switch and 3 push buttons
connected to SoC GPIOs, meant to be used as general-purpose test keys.
Add a corresponding node in DT, mapping (semi-randomly) the DIP switch
to keys 1-4 and the push buttons to keys A-C.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 63 ++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
index f35e96ca7d60..8e511fab880c 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
+++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
@@ -11,6 +11,7 @@
 /dts-v1/;
 #include "r8a7796.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 
 / {
 	model = "Renesas Salvator-X board based on r8a7796";
@@ -31,6 +32,63 @@
 		reg = <0x0 0x48000000 0x0 0x78000000>;
 	};
 
+	keys {
+		compatible = "gpio-keys";
+
+		pinctrl-0 = <&keys_pins>;
+		pinctrl-names = "default";
+
+		key-1 {
+			gpios = <&gpio5 17 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_1>;
+			label = "SW4-1";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-2 {
+			gpios = <&gpio5 20 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_2>;
+			label = "SW4-2";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-3 {
+			gpios = <&gpio5 22 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_3>;
+			label = "SW4-3";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-4 {
+			gpios = <&gpio5 23 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_4>;
+			label = "SW4-4";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-a {
+			gpios = <&gpio6 11 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_A>;
+			label = "TSW0";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-b {
+			gpios = <&gpio6 12 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_B>;
+			label = "TSW1";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+		key-c {
+			gpios = <&gpio6 13 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_C>;
+			label = "TSW2";
+			wakeup-source;
+			debounce-interval = <20>;
+		};
+	};
+
 	reg_1p8v: regulator0 {
 		compatible = "regulator-fixed";
 		regulator-name = "fixed-1.8V";
@@ -116,6 +174,11 @@
 		function = "i2c2";
 	};
 
+	keys_pins: keys {
+		pins = "GP_5_17", "GP_5_20", "GP_5_22";
+		bias-pull-up;
+	};
+
 	sdhi0_pins: sd0 {
 		groups = "sdhi0_data4", "sdhi0_ctrl";
 		function = "sdhi0";
-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Nov 14, 2016 at 2:07 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> This simple series adds GPIO keys support to both the H3- and M3-W-based
> Salvator-X boards. Please see individual patches for details.

Thanks for your series!

The main reason I haven't sent out a similar series yet is because the GPIOs
used for the 3 push buttons are shared with the 3 user LEDs. For each of them,
you have to choose at DT time if you want to use them as buttons or as LEDs.

On ULCB, the same issue is present. For those, we settled on 1 key and 2
LEDs...

Looking forward to more comments...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Laurent Pinchart]

Hi Geert,

On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
> On Mon, Nov 14, 2016 at 2:07 PM, Laurent Pinchart wrote:
> > This simple series adds GPIO keys support to both the H3- and M3-W-based
> > Salvator-X boards. Please see individual patches for details.
> 
> Thanks for your series!
> 
> The main reason I haven't sent out a similar series yet is because the GPIOs
> used for the 3 push buttons are shared with the 3 user LEDs. For each of
> them, you have to choose at DT time if you want to use them as buttons or
> as LEDs.
> 
> On ULCB, the same issue is present. For those, we settled on 1 key and 2
> LEDs...
> 
> Looking forward to more comments...

In theory the GPIOs could be shared by the gpio-keys and LED drivers in open-
drain mode. I'm not sure the GPIO subsystem supports that though.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
>> The main reason I haven't sent out a similar series yet is because the GPIOs
>> used for the 3 push buttons are shared with the 3 user LEDs. For each of
>> them, you have to choose at DT time if you want to use them as buttons or
>> as LEDs.
>>
>> On ULCB, the same issue is present. For those, we settled on 1 key and 2
>> LEDs...
>>
>> Looking forward to more comments...
>
> In theory the GPIOs could be shared by the gpio-keys and LED drivers in open-
> drain mode. I'm not sure the GPIO subsystem supports that though.

Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons) and
output (LEDs)". The result wasn't pretty...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Laurent Pinchart]

Hi Geert,

On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
> > On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
> >> The main reason I haven't sent out a similar series yet is because the
> >> GPIOs used for the 3 push buttons are shared with the 3 user LEDs. For
> >> each of them, you have to choose at DT time if you want to use them as
> >> buttons or as LEDs.
> >> 
> >> On ULCB, the same issue is present. For those, we settled on 1 key and 2
> >> LEDs...
> >> 
> >> Looking forward to more comments...
> > 
> > In theory the GPIOs could be shared by the gpio-keys and LED drivers in
> > open- drain mode. I'm not sure the GPIO subsystem supports that though.
> 
> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons) and
> output (LEDs)". The result wasn't pretty...

Wasn't it ? Linus basically told you to use open-drain GPIOs and fix the GPIO 
driver in case it can't read the value of GPIOs configured as output in open-
drain mode. If didn't shoot the idea down.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Geert Uytterhoeven]

Hi Laurent,

On Mon, Nov 14, 2016 at 5:44 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
>> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
>> > On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
>> >> The main reason I haven't sent out a similar series yet is because the
>> >> GPIOs used for the 3 push buttons are shared with the 3 user LEDs. For
>> >> each of them, you have to choose at DT time if you want to use them as
>> >> buttons or as LEDs.
>> >>
>> >> On ULCB, the same issue is present. For those, we settled on 1 key and 2
>> >> LEDs...
>> >>
>> >> Looking forward to more comments...
>> >
>> > In theory the GPIOs could be shared by the gpio-keys and LED drivers in
>> > open- drain mode. I'm not sure the GPIO subsystem supports that though.
>>
>> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons) and
>> output (LEDs)". The result wasn't pretty...
>
> Wasn't it ? Linus basically told you to use open-drain GPIOs and fix the GPIO
> driver in case it can't read the value of GPIOs configured as output in open-
> drain mode. If didn't shoot the idea down.

If I'm not mistaken, the R-Car GPIO block does not support open-drain GPIO.
Even if it would support it, you cannot read the GPIO while actively
pulling it down.
Hence you have to time-multiplex the GPIO to use both LEDs and buttons,
through switching between pulling down and not pulling down (or between
output and input, which is what I did).

Apart from that, there's also the discrepancy between hardware description
(the GPIO is connected to both buttons and LEDs, hence it should be described
that way in DT) and user policy (the user wants to use e.g. the first GPIO as a
button, and the second GPIO as an LED).

If you have ideas to solve these 2  issues, I'm happy  to hear your thoughts!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Laurent Pinchart]

Hi Geert,

On Monday 14 Nov 2016 20:47:03 Geert Uytterhoeven wrote:
> On Mon, Nov 14, 2016 at 5:44 PM, Laurent Pinchart wrote:
> > On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
> >> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
> >>> On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
> >>>> The main reason I haven't sent out a similar series yet is because the
> >>>> GPIOs used for the 3 push buttons are shared with the 3 user LEDs. For
> >>>> each of them, you have to choose at DT time if you want to use them as
> >>>> buttons or as LEDs.
> >>>> 
> >>>> On ULCB, the same issue is present. For those, we settled on 1 key and
> >>>> 2 LEDs...
> >>>> 
> >>>> Looking forward to more comments...
> >>> 
> >>> In theory the GPIOs could be shared by the gpio-keys and LED drivers in
> >>> open- drain mode. I'm not sure the GPIO subsystem supports that though.
> >> 
> >> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons) and
> >> output (LEDs)". The result wasn't pretty...
> > 
> > Wasn't it ? Linus basically told you to use open-drain GPIOs and fix the
> > GPIO driver in case it can't read the value of GPIOs configured as output
> > in open- drain mode. If didn't shoot the idea down.
> 
> If I'm not mistaken, the R-Car GPIO block does not support open-drain GPIO.

Not as such, but we can switch between input and output low, which is 
basically open-drain.

> Even if it would support it, you cannot read the GPIO while actively
> pulling it down.

When actively driving it low you know the value is 0. Only when driving it 
open do you need to read the value back, and that's easily implemented as the 
hardware will be configured in input mode then.

> Hence you have to time-multiplex the GPIO to use both LEDs and buttons,
> through switching between pulling down and not pulling down (or between
> output and input, which is what I did).

No, if you want to use both, you should configure the I/O in open-drain, in 
which case you have two options:

- Turn the LED on by driving the I/O "high", meaning leaving it floating. The 
pull-up resistor will turn the MOSFET on, the LED will be lit. When the 
corresponding button is pressed the I/O will be connected to GND, turning the 
LED off, and signalling the input.

- Turn the LED off by driving the I/O low. Pressing the button won't have any 
effect. We need in this case to ignore the input value, which could be done by 
disabling the I/O interrupt (or just ignoring it).

> Apart from that, there's also the discrepancy between hardware description
> (the GPIO is connected to both buttons and LEDs, hence it should be
> described that way in DT)

Correct, this is where a framework change is needed if we want to allow both 
the GPIO keyboard and LED drivers to request the GPIO.

> and user policy (the user wants to use e.g. the first GPIO as a button, and
> the second GPIO as an LED).

That's easy to do, the user will just need to turn the first LED on, allowing 
full button operation on the I/O.

> If you have ideas to solve these 2  issues, I'm happy  to hear your
> thoughts!

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Nov 15, 2016 at 12:59 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 14 Nov 2016 20:47:03 Geert Uytterhoeven wrote:
>> On Mon, Nov 14, 2016 at 5:44 PM, Laurent Pinchart wrote:
>> > On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
>> >> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
>> >>> On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
>> >>>> The main reason I haven't sent out a similar series yet is because the
>> >>>> GPIOs used for the 3 push buttons are shared with the 3 user LEDs. For
>> >>>> each of them, you have to choose at DT time if you want to use them as
>> >>>> buttons or as LEDs.
>> >>>>
>> >>>> On ULCB, the same issue is present. For those, we settled on 1 key and
>> >>>> 2 LEDs...
>> >>>>
>> >>>> Looking forward to more comments...
>> >>>
>> >>> In theory the GPIOs could be shared by the gpio-keys and LED drivers in
>> >>> open- drain mode. I'm not sure the GPIO subsystem supports that though.
>> >>
>> >> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons) and
>> >> output (LEDs)". The result wasn't pretty...
>> >
>> > Wasn't it ? Linus basically told you to use open-drain GPIOs and fix the
>> > GPIO driver in case it can't read the value of GPIOs configured as output
>> > in open- drain mode. If didn't shoot the idea down.
>>
>> If I'm not mistaken, the R-Car GPIO block does not support open-drain GPIO.
>
> Not as such, but we can switch between input and output low, which is
> basically open-drain.
>
>> Even if it would support it, you cannot read the GPIO while actively
>> pulling it down.
>
> When actively driving it low you know the value is 0. Only when driving it
> open do you need to read the value back, and that's easily implemented as the
> hardware will be configured in input mode then.
>
>> Hence you have to time-multiplex the GPIO to use both LEDs and buttons,
>> through switching between pulling down and not pulling down (or between
>> output and input, which is what I did).
>
> No, if you want to use both, you should configure the I/O in open-drain, in
> which case you have two options:
>
> - Turn the LED on by driving the I/O "high", meaning leaving it floating. The
> pull-up resistor will turn the MOSFET on, the LED will be lit. When the
> corresponding button is pressed the I/O will be connected to GND, turning the
> LED off, and signalling the input.
>
> - Turn the LED off by driving the I/O low. Pressing the button won't have any
> effect. We need in this case to ignore the input value, which could be done by

Right. I tried to have both ;-)

> disabling the I/O interrupt (or just ignoring it).

IIRC, the R-Car GPIO block will send interrupts in output mode only.

>> Apart from that, there's also the discrepancy between hardware description
>> (the GPIO is connected to both buttons and LEDs, hence it should be
>> described that way in DT)
>
> Correct, this is where a framework change is needed if we want to allow both
> the GPIO keyboard and LED drivers to request the GPIO.
>
>> and user policy (the user wants to use e.g. the first GPIO as a button, and
>> the second GPIO as an LED).
>
> That's easy to do, the user will just need to turn the first LED on, allowing
> full button operation on the I/O.

Thanks, didn't think of that.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Laurent Pinchart]

Hi Geert,

(CC'ing Linus)

On Tuesday 15 Nov 2016 08:58:18 Geert Uytterhoeven wrote:
> On Tue, Nov 15, 2016 at 12:59 AM, Laurent Pinchart wrote:
> > On Monday 14 Nov 2016 20:47:03 Geert Uytterhoeven wrote:
> >> On Mon, Nov 14, 2016 at 5:44 PM, Laurent Pinchart wrote:
> >>> On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
> >>>> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
> >>>>> On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
> >>>>>> The main reason I haven't sent out a similar series yet is because
> >>>>>> the GPIOs used for the 3 push buttons are shared with the 3 user
> >>>>>> LEDs. For each of them, you have to choose at DT time if you want to
> >>>>>> use them as buttons or as LEDs.
> >>>>>> 
> >>>>>> On ULCB, the same issue is present. For those, we settled on 1 key
> >>>>>> and 2 LEDs...
> >>>>>> 
> >>>>>> Looking forward to more comments...
> >>>>> 
> >>>>> In theory the GPIOs could be shared by the gpio-keys and LED drivers
> >>>>> in open-drain mode. I'm not sure the GPIO subsystem supports that
> >>>>> though.
> >>>> 
> >>>> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons)
> >>>> and output (LEDs)". The result wasn't pretty...
> >>> 
> >>> Wasn't it ? Linus basically told you to use open-drain GPIOs and fix
> >>> the GPIO driver in case it can't read the value of GPIOs configured as
> >>> output in open-drain mode. If didn't shoot the idea down.
> >> 
> >> If I'm not mistaken, the R-Car GPIO block does not support open-drain
> >> GPIO.
> > 
> > Not as such, but we can switch between input and output low, which is
> > basically open-drain.
> > 
> >> Even if it would support it, you cannot read the GPIO while actively
> >> pulling it down.
> > 
> > When actively driving it low you know the value is 0. Only when driving it
> > open do you need to read the value back, and that's easily implemented as
> > the hardware will be configured in input mode then.
> > 
> >> Hence you have to time-multiplex the GPIO to use both LEDs and buttons,
> >> through switching between pulling down and not pulling down (or between
> >> output and input, which is what I did).
> > 
> > No, if you want to use both, you should configure the I/O in open-drain,
> > in which case you have two options:
> > 
> > - Turn the LED on by driving the I/O "high", meaning leaving it floating.
> > The pull-up resistor will turn the MOSFET on, the LED will be lit. When
> > the corresponding button is pressed the I/O will be connected to GND,
> > turning the LED off, and signalling the input.
> > 
> > - Turn the LED off by driving the I/O low. Pressing the button won't have
> > any effect. We need in this case to ignore the input value, which could
> > be done by
>
> Right. I tried to have both ;-)

I'm sorry Dave, I'm afraid you can't do that :-)

> > disabling the I/O interrupt (or just ignoring it).
> 
> IIRC, the R-Car GPIO block will send interrupts in output mode only.

Do you mean input mode only ?

> >> Apart from that, there's also the discrepancy between hardware
> >> description (the GPIO is connected to both buttons and LEDs, hence it
> >> should be described that way in DT)
> > 
> > Correct, this is where a framework change is needed if we want to allow
> > both the GPIO keyboard and LED drivers to request the GPIO.

Linus, would this be acceptable to you ?

> >> and user policy (the user wants to use e.g. the first GPIO as a button,
> >> and the second GPIO as an LED).
> > 
> > That's easy to do, the user will just need to turn the first LED on,
> > allowing full button operation on the I/O.
> 
> Thanks, didn't think of that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Geert Uytterhoeven]

On Tue, Nov 15, 2016 at 3:26 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> > disabling the I/O interrupt (or just ignoring it).
>>
>> IIRC, the R-Car GPIO block will send interrupts in output mode only.
>
> Do you mean input mode only ?

Yes, sorry (still suffering a bit from my cold).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Laurent Pinchart]

On Tuesday 15 Nov 2016 16:26:11 Laurent Pinchart wrote:
> Hi Geert,
> 
> (CC'ing Linus)

With Linus' e-mail address fixed. I'll blame my e-mail client that uses 
incorrect addresses from old e-mails as the top auto-completion options :-/

> On Tuesday 15 Nov 2016 08:58:18 Geert Uytterhoeven wrote:
> > On Tue, Nov 15, 2016 at 12:59 AM, Laurent Pinchart wrote:
> >> On Monday 14 Nov 2016 20:47:03 Geert Uytterhoeven wrote:
> >>> On Mon, Nov 14, 2016 at 5:44 PM, Laurent Pinchart wrote:
> >>>> On Monday 14 Nov 2016 14:47:00 Geert Uytterhoeven wrote:
> >>>>> On Mon, Nov 14, 2016 at 2:41 PM, Laurent Pinchart wrote:
> >>>>>> On Monday 14 Nov 2016 14:35:26 Geert Uytterhoeven wrote:
> >>>>>>> The main reason I haven't sent out a similar series yet is because
> >>>>>>> the GPIOs used for the 3 push buttons are shared with the 3 user
> >>>>>>> LEDs. For each of them, you have to choose at DT time if you want
> >>>>>>> to use them as buttons or as LEDs.
> >>>>>>> 
> >>>>>>> On ULCB, the same issue is present. For those, we settled on 1 key
> >>>>>>> and 2 LEDs...
> >>>>>>> 
> >>>>>>> Looking forward to more comments...
> >>>>>> 
> >>>>>> In theory the GPIOs could be shared by the gpio-keys and LED drivers
> >>>>>> in open-drain mode. I'm not sure the GPIO subsystem supports that
> >>>>>> though.
> >>>>> 
> >>>>> Been there, done that, cfr. "[RFD] Sharing GPIOs for input (buttons)
> >>>>> and output (LEDs)". The result wasn't pretty...
> >>>> 
> >>>> Wasn't it ? Linus basically told you to use open-drain GPIOs and fix
> >>>> the GPIO driver in case it can't read the value of GPIOs configured as
> >>>> output in open-drain mode. If didn't shoot the idea down.
> >>> 
> >>> If I'm not mistaken, the R-Car GPIO block does not support open-drain
> >>> GPIO.
> >> 
> >> Not as such, but we can switch between input and output low, which is
> >> basically open-drain.
> >> 
> >>> Even if it would support it, you cannot read the GPIO while actively
> >>> pulling it down.
> >> 
> >> When actively driving it low you know the value is 0. Only when driving
> >> it open do you need to read the value back, and that's easily implemented
> >> as the hardware will be configured in input mode then.
> >> 
> >>> Hence you have to time-multiplex the GPIO to use both LEDs and buttons,
> >>> through switching between pulling down and not pulling down (or between
> >>> output and input, which is what I did).
> >> 
> >> No, if you want to use both, you should configure the I/O in open-drain,
> >> in which case you have two options:
> >> 
> >> - Turn the LED on by driving the I/O "high", meaning leaving it
> >> floating. The pull-up resistor will turn the MOSFET on, the LED will be
> >> lit. When the corresponding button is pressed the I/O will be connected
> >> to GND, turning the LED off, and signalling the input.
> >> 
> >> - Turn the LED off by driving the I/O low. Pressing the button won't
> >> have any effect. We need in this case to ignore the input value, which
> >> could be done by
> > 
> > Right. I tried to have both ;-)
> 
> I'm sorry Dave, I'm afraid you can't do that :-)
> 
> >> disabling the I/O interrupt (or just ignoring it).
> > 
> > IIRC, the R-Car GPIO block will send interrupts in output mode only.
> 
> Do you mean input mode only ?
> 
> >>> Apart from that, there's also the discrepancy between hardware
> >>> description (the GPIO is connected to both buttons and LEDs, hence it
> >>> should be described that way in DT)
> >> 
> >> Correct, this is where a framework change is needed if we want to allow
> >> both the GPIO keyboard and LED drivers to request the GPIO.
> 
> Linus, would this be acceptable to you ?
> 
> >>> and user policy (the user wants to use e.g. the first GPIO as a button,
> >>> and the second GPIO as an LED).
> >> 
> >> That's easy to do, the user will just need to turn the first LED on,
> >> allowing full button operation on the I/O.
> > 
> > Thanks, didn't think of that.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/2] Salvator-X: Add GPIO keys support

[Linus Walleij]

On Tue, Nov 15, 2016 at 3:28 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

>> >>> Apart from that, there's also the discrepancy between hardware
>> >>> description (the GPIO is connected to both buttons and LEDs, hence it
>> >>> should be described that way in DT)
>> >>
>> >> Correct, this is where a framework change is needed if we want to allow
>> >> both the GPIO keyboard and LED drivers to request the GPIO.
>>
>> Linus, would this be acceptable to you ?

Yes, if:

- It is solved in a generic way so that there is a call like
gpiod_get_nonexclusive()
  or similar to get a second handle on a GPIOd someone else is already using.

- It is cross-coordinated with the regulator people who have exactly the same
  problem for fixed GPIO regulators and which is why that code looks like hell.

Apart from that, we generally need a way to access LEDs as resources with
foo-leds = <&led_foo>; in the device tree just like we can get GPIOs and I
hope someone is working on that for this thing... triggers is such a hack.

Yours,
Linus Walleij