| Message ID | 1457294038-14243-7-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
> Le 6 mars 2016 à 20:53, Andreas Färber <afaerber@suse.de> a écrit : > > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > v2 -> v3: > * Adopted wakeup-source instead of gpio-key,wakeup (Julien) > * Dropped gpio-keys #address-cells and #size-cells properties (Julien) > * Dropped power button reg property (Julien) > * Adopted KEY_POWER (Julien) > * Fixed power button pinctrl pull setting (Julien) > > v2: New > > arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts > index 098be3700a6f..7036b49c9206 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts > @@ -42,6 +42,7 @@ > > /dts-v1/; > #include "rk3368.dtsi" > +#include <dt-bindings/input/input.h> > > / { > model = "GeekBox"; > @@ -70,6 +71,19 @@ > pinctrl-0 = <&ir_int>; > }; > > + keys: gpio-keys { I think you don't need the "keys" label, because there’s no phandle that refers to this. > + compatible = "gpio-keys"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pwr_key>; > + > + button@0 { Here you should use "power" instead of "button@0". > + gpios = <&gpio0 2 GPIO_ACTIVE_LOW>; > + label = "GPIO Power"; > + linux,code = <KEY_POWER>; According to Documentation/input/event-codes.txt, there’s a special event type for the power button. Should we use it here for that purpose? linux,input-type = <EV_PWR> > + wakeup-source; > + }; > + }; > + > leds: gpio-leds { > compatible = "gpio-leds"; > > @@ -265,6 +279,12 @@ > }; > }; > > + keys { > + pwr_key: pwr-key { > + rockchip,pins = <0 2 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > + > pmic { > pmic_sleep: pmic-sleep { > rockchip,pins = <0 0 RK_FUNC_2 &pcfg_pull_none>; > -- > 2.6.2 >
> Le 6 mars 2016 à 20:53, Andreas Färber <afaerber@suse.de> a écrit : > > Signed-off-by: Andreas Färber <afaerber@suse.de> > --- > v2 -> v3: > * Adopted wakeup-source instead of gpio-key,wakeup (Julien) > * Dropped gpio-keys #address-cells and #size-cells properties (Julien) > * Dropped power button reg property (Julien) > * Adopted KEY_POWER (Julien) > * Fixed power button pinctrl pull setting (Julien) > > v2: New > > arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts > index 098be3700a6f..7036b49c9206 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts > @@ -42,6 +42,7 @@ > > /dts-v1/; > #include "rk3368.dtsi" > +#include <dt-bindings/input/input.h> > > / { > model = "GeekBox"; > @@ -70,6 +71,19 @@ > pinctrl-0 = <&ir_int>; > }; > > + keys: gpio-keys { I think you don't need the "keys" label, because there’s no phandle that refers to this. > + compatible = "gpio-keys"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pwr_key>; > + > + button@0 { Here you should use "power" instead of "button@0". > + gpios = <&gpio0 2 GPIO_ACTIVE_LOW>; > + label = "GPIO Power"; > + linux,code = <KEY_POWER>; According to Documentation/input/event-codes.txt, there’s a special event type for the power button. Should we use it here for that purpose? linux,input-type = <EV_PWR> > + wakeup-source; > + }; > + }; > + > leds: gpio-leds { > compatible = "gpio-leds"; > > @@ -265,6 +279,12 @@ > }; > }; > > + keys { > + pwr_key: pwr-key { > + rockchip,pins = <0 2 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > + > pmic { > pmic_sleep: pmic-sleep { > rockchip,pins = <0 0 RK_FUNC_2 &pcfg_pull_none>; > -- > 2.6.2 >
Am 11.03.2016 um 00:04 schrieb Julien Chauveau: >> Le 6 mars 2016 à 20:53, Andreas Färber <afaerber@suse.de> a écrit : >> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> --- >> v2 -> v3: >> * Adopted wakeup-source instead of gpio-key,wakeup (Julien) >> * Dropped gpio-keys #address-cells and #size-cells properties (Julien) >> * Dropped power button reg property (Julien) >> * Adopted KEY_POWER (Julien) >> * Fixed power button pinctrl pull setting (Julien) >> >> v2: New >> >> arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts >> index 098be3700a6f..7036b49c9206 100644 >> --- a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts >> +++ b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts >> @@ -42,6 +42,7 @@ >> >> /dts-v1/; >> #include "rk3368.dtsi" >> +#include <dt-bindings/input/input.h> >> >> / { >> model = "GeekBox"; >> @@ -70,6 +71,19 @@ >> pinctrl-0 = <&ir_int>; >> }; >> >> + keys: gpio-keys { > > I think you don't need the "keys" label, because there’s no phandle that refers to this. As discussed elsewhere, there are four additional keys on the Landingship (you proposed as sub-node names key1-key4). I prefer preparing the label now over adding it in a later patch. >> + compatible = "gpio-keys"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pwr_key>; >> + >> + button@0 { > > Here you should use "power" instead of "button@0". Done. >> + gpios = <&gpio0 2 GPIO_ACTIVE_LOW>; >> + label = "GPIO Power"; >> + linux,code = <KEY_POWER>; > > According to Documentation/input/event-codes.txt, there’s a special event type for the power button. > Should we use it here for that purpose? > > linux,input-type = <EV_PWR> The other RK3368 boards don't, so unless you can give a justification to convert all boards yet again and test how this makes a difference, I'd rather not do experiments here but leave that to someone who knows what they're doing and then do it consistently... Thanks for the detailed review, Andreas >> + wakeup-source; >> + }; >> + }; >> + >> leds: gpio-leds { >> compatible = "gpio-leds"; >> >> @@ -265,6 +279,12 @@ >> }; >> }; >> >> + keys { >> + pwr_key: pwr-key { >> + rockchip,pins = <0 2 RK_FUNC_GPIO &pcfg_pull_none>; >> + }; >> + }; >> + >> pmic { >> pmic_sleep: pmic-sleep { >> rockchip,pins = <0 0 RK_FUNC_2 &pcfg_pull_none>;
Am 16.03.2016 um 11:58 schrieb Andreas Färber: > Am 11.03.2016 um 00:04 schrieb Julien Chauveau: >>> @@ -70,6 +71,19 @@ >>> pinctrl-0 = <&ir_int>; >>> }; >>> >>> + keys: gpio-keys { [...] >>> + compatible = "gpio-keys"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&pwr_key>; >>> + >>> + button@0 { [...] >>> + gpios = <&gpio0 2 GPIO_ACTIVE_LOW>; >>> + label = "GPIO Power"; >>> + linux,code = <KEY_POWER>; >> >> According to Documentation/input/event-codes.txt, there’s a special event type for the power button. >> Should we use it here for that purpose? >> >> linux,input-type = <EV_PWR> > > The other RK3368 boards don't, so unless you can give a justification to > convert all boards yet again and test how this makes a difference, I'd > rather not do experiments here but leave that to someone who knows what > they're doing and then do it consistently... For the record here's an evtest log: geekbox:~ # evtest No device specified, trying to scan all of /dev/input/event* Available devices: /dev/input/event0: gpio_ir_recv /dev/input/event1: MCE IR Keyboard/Mouse (gpio-rc-recv) /dev/input/event2: gpio-keys Select the device event number [0-2]: 2 Input driver version is 1.0.1 Input device ID: bus 0x19 vendor 0x1 product 0x1 version 0x100 Input device name: "gpio-keys" Supported events: Event type 0 (EV_SYN) Event type 1 (EV_KEY) Event code 116 (KEY_POWER) Properties: Testing ... (interrupt to exit) Event: time 1458136008.850429, type 1 (EV_KEY), code 116 (KEY_POWER), value 1 Event: time 1458136008.850429, -------------- SYN_REPORT ------------ systemd then goes on to shut down the system cleanly. Regards, Andreas
diff --git a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts index 098be3700a6f..7036b49c9206 100644 --- a/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts +++ b/arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts @@ -42,6 +42,7 @@ /dts-v1/; #include "rk3368.dtsi" +#include <dt-bindings/input/input.h> / { model = "GeekBox"; @@ -70,6 +71,19 @@ pinctrl-0 = <&ir_int>; }; + keys: gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&pwr_key>; + + button@0 { + gpios = <&gpio0 2 GPIO_ACTIVE_LOW>; + label = "GPIO Power"; + linux,code = <KEY_POWER>; + wakeup-source; + }; + }; + leds: gpio-leds { compatible = "gpio-leds"; @@ -265,6 +279,12 @@ }; }; + keys { + pwr_key: pwr-key { + rockchip,pins = <0 2 RK_FUNC_GPIO &pcfg_pull_none>; + }; + }; + pmic { pmic_sleep: pmic-sleep { rockchip,pins = <0 0 RK_FUNC_2 &pcfg_pull_none>;
Signed-off-by: Andreas Färber <afaerber@suse.de> --- v2 -> v3: * Adopted wakeup-source instead of gpio-key,wakeup (Julien) * Dropped gpio-keys #address-cells and #size-cells properties (Julien) * Dropped power button reg property (Julien) * Adopted KEY_POWER (Julien) * Fixed power button pinctrl pull setting (Julien) v2: New arch/arm64/boot/dts/rockchip/rk3368-geekbox.dts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)