Hi Kieran, CC input On Wed, Sep 22, 2021 at 10:30 PM Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> wrote: > Add support for SW46-1 and SW46-2 as switches using the gpio-keys > framework. > > Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx 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