linux-kernel - Re: [PATCH] arm64: dts: renesas: r8a779a0: falcon-cpu: Add SW46 switch support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAMuHMdULHnztv=7i1b1x9BEsO8pu=J3Af_Qx7=CzD3qJhYRNBA@mail.gmail.com>
Date:   Thu, 23 Sep 2021 09:32:00 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Kieran Bingham <kieran.bingham@...asonboard.com>
Cc:     Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Magnus Damm <magnus.damm@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        linux-input <linux-input@...r.kernel.org>
Subject: Re: [PATCH] arm64: dts: renesas: r8a779a0: falcon-cpu: Add SW46
 switch support

Hi Kieran,

CC input

On Wed, Sep 22, 2021 at 10:30 PM Kieran Bingham
<kieran.bingham@...asonboard.com> wrote:
> Add support for SW46-1 and SW46-2 as switches using the gpio-keys
> framework.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@...asonboard.com>

Thanks for your patch!

> ---
>
> SW_LID and SW_DOCK are selected as low-impact switch events for the
> default configuration. Would SW_RFKILL_ALL, and SW_MUTE_DEVICE be
> preferred as more 'functional' defaults? (I have otherwise avoided these
> to hopefully prevent unwanted / undocumented effects occuring on
> development hardware running a full interface which may parse these)
>
> I'd expect them to be overridden by any platform using them anyway.

That's a good question

BTW, I'm happy you brought this up.  I discovered EV_SW only
recently, and had just started wondering whether we should use it
for the various slide switches on Renesas R-Car Gen2 and Gen3 boards,
which are modelled using the default EV_KEY and KEY_[1-4].

I see several DTS files using EV_SW (or hardcoded 5) with KEY_*
codes instead of EV_* codes, so perhaps KEY_A or KEY_B would be
suited better, to avoid strange effects? But SW_LID (and KEY_RESERVED)
seem to be quite popular, too.

Any input^Wgood advice from the input people? TIA!

> --- a/arch/arm64/boot/dts/renesas/r8a779a0-falcon-cpu.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779a0-falcon-cpu.dtsi
> @@ -52,6 +52,24 @@ keys {
>                 pinctrl-0 = <&keys_pins>;
>                 pinctrl-names = "default";
>
> +               sw-1 {
> +                       gpios = <&gpio1 28 GPIO_ACTIVE_LOW>;
> +                       linux,code = <SW_LID>;
> +                       linux,input-type = <EV_SW>;
> +                       label = "SW46-1";
> +                       wakeup-source;
> +                       debounce-interval = <20>;
> +               };
> +
> +               sw-2 {
> +                       gpios = <&gpio1 29 GPIO_ACTIVE_LOW>;
> +                       linux,code = <SW_DOCK>;
> +                       linux,input-type = <EV_SW>;
> +                       label = "SW46-2";
> +                       wakeup-source;
> +                       debounce-interval = <20>;
> +               };
> +
>                 key-1 {
>                         gpios = <&gpio6 18 GPIO_ACTIVE_LOW>;
>                         linux,code = <KEY_1>;

Looks good to me.

> @@ -193,7 +211,8 @@ i2c6_pins: i2c6 {
>         };
>
>         keys_pins: keys {
> -               pins = "GP_6_18", "GP_6_19", "GP_6_20";
> +               pins = "GP_1_28", "GP_1_29",
> +                      "GP_6_18", "GP_6_19", "GP_6_20";
>                 bias-pull-up;
>         };

This part is not needed, as the GPIOs connected to the slide switches
have external pull-up resistors (unlike the GPIOs connected to the
push switches, which are driven low by open-drain buffers, without
external pull-up resistors).

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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