* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 8:01 ` Anand Moon 0 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 8:01 UTC (permalink / raw) To: Kukjin Kim, Krzysztof Kozlowski, Javier Martinez Canillas, Marek Szyprowski, Anand Moon Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel Add support for gpio-based button on Odroid-XU3 boards for reboot/poweroff feature. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- changes rebase based on linux next-20160222. Tested on Odroid-XU4 dmesg output. [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 --- arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi index 1bd507b..db9770b 100644 --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi @@ -11,6 +11,7 @@ */ #include <dt-bindings/clock/samsung,s2mps11.h> +#include <dt-bindings/input/input.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/sound/samsung-i2s.h> @@ -54,6 +55,22 @@ #cooling-cells = <2>; cooling-levels = <0 130 170 230>; }; + + gpio_keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&gpio_power_key>; + + power_key { + interrupt-parent = <&gpx0>; + interrupts = <3 IRQ_TYPE_NONE>; + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>; + label = "power key"; + debounce-interval = <10>; + wakeup-source; + }; + }; }; &clock_audss { @@ -362,6 +379,11 @@ }; &pinctrl_0 { + gpio_power_key: power_key { + samsung,pins = "gpx0-3"; + samsung,pin-pud = <0>; + }; + hdmi_hpd_irq: hdmi-hpd-irq { samsung,pins = "gpx3-7"; samsung,pin-function = <0>; -- 2.1.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 8:01 ` Anand Moon 0 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 8:01 UTC (permalink / raw) To: linux-arm-kernel Add support for gpio-based button on Odroid-XU3 boards for reboot/poweroff feature. Signed-off-by: Anand Moon <linux.amoon@gmail.com> --- changes rebase based on linux next-20160222. Tested on Odroid-XU4 dmesg output. [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 --- arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi index 1bd507b..db9770b 100644 --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi @@ -11,6 +11,7 @@ */ #include <dt-bindings/clock/samsung,s2mps11.h> +#include <dt-bindings/input/input.h> #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/sound/samsung-i2s.h> @@ -54,6 +55,22 @@ #cooling-cells = <2>; cooling-levels = <0 130 170 230>; }; + + gpio_keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&gpio_power_key>; + + power_key { + interrupt-parent = <&gpx0>; + interrupts = <3 IRQ_TYPE_NONE>; + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; + linux,code = <KEY_POWER>; + label = "power key"; + debounce-interval = <10>; + wakeup-source; + }; + }; }; &clock_audss { @@ -362,6 +379,11 @@ }; &pinctrl_0 { + gpio_power_key: power_key { + samsung,pins = "gpx0-3"; + samsung,pin-pud = <0>; + }; + hdmi_hpd_irq: hdmi-hpd-irq { samsung,pins = "gpx3-7"; samsung,pin-function = <0>; -- 2.1.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 8:01 ` Anand Moon @ 2016-02-23 8:21 ` Krzysztof Kozlowski -1 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 8:21 UTC (permalink / raw) To: Anand Moon, Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel On 23.02.2016 17:01, Anand Moon wrote: > Add support for gpio-based button on Odroid-XU3 boards > for reboot/poweroff feature. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > changes rebase based on linux next-20160222. > > Tested on Odroid-XU4 > > dmesg output. > [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) > [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations > [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 > --- > arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > index 1bd507b..db9770b 100644 > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > @@ -11,6 +11,7 @@ > */ > > #include <dt-bindings/clock/samsung,s2mps11.h> > +#include <dt-bindings/input/input.h> > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/sound/samsung-i2s.h> > @@ -54,6 +55,22 @@ > #cooling-cells = <2>; > cooling-levels = <0 130 170 230>; > }; > + > + gpio_keys { > + compatible = "gpio-keys"; > + pinctrl-names = "default"; > + pinctrl-0 = <&gpio_power_key>; > + > + power_key { > + interrupt-parent = <&gpx0>; > + interrupts = <3 IRQ_TYPE_NONE>; Hmmm.... why you specify the interrupts? > + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > + linux,code = <KEY_POWER>; > + label = "power key"; Just "power". > + debounce-interval = <10>; > + wakeup-source; > + }; > + }; > }; > > &clock_audss { > @@ -362,6 +379,11 @@ > }; > > &pinctrl_0 { > + gpio_power_key: power_key { The naming is mixed... Everything is GPIO here so don't add such prefix. Underscores only in label, not in name of node. power_key_irq: power-key-irq { > + samsung,pins = "gpx0-3"; > + samsung,pin-pud = <0>; Don't you want to set specific pin function? And what about drive strength? Best regards, Krzysztof > + }; > + > hdmi_hpd_irq: hdmi-hpd-irq { > samsung,pins = "gpx3-7"; > samsung,pin-function = <0>; > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 8:21 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 8:21 UTC (permalink / raw) To: linux-arm-kernel On 23.02.2016 17:01, Anand Moon wrote: > Add support for gpio-based button on Odroid-XU3 boards > for reboot/poweroff feature. > > Signed-off-by: Anand Moon <linux.amoon@gmail.com> > --- > changes rebase based on linux next-20160222. > > Tested on Odroid-XU4 > > dmesg output. > [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) > [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations > [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 > --- > arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > index 1bd507b..db9770b 100644 > --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi > @@ -11,6 +11,7 @@ > */ > > #include <dt-bindings/clock/samsung,s2mps11.h> > +#include <dt-bindings/input/input.h> > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/gpio/gpio.h> > #include <dt-bindings/sound/samsung-i2s.h> > @@ -54,6 +55,22 @@ > #cooling-cells = <2>; > cooling-levels = <0 130 170 230>; > }; > + > + gpio_keys { > + compatible = "gpio-keys"; > + pinctrl-names = "default"; > + pinctrl-0 = <&gpio_power_key>; > + > + power_key { > + interrupt-parent = <&gpx0>; > + interrupts = <3 IRQ_TYPE_NONE>; Hmmm.... why you specify the interrupts? > + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > + linux,code = <KEY_POWER>; > + label = "power key"; Just "power". > + debounce-interval = <10>; > + wakeup-source; > + }; > + }; > }; > > &clock_audss { > @@ -362,6 +379,11 @@ > }; > > &pinctrl_0 { > + gpio_power_key: power_key { The naming is mixed... Everything is GPIO here so don't add such prefix. Underscores only in label, not in name of node. power_key_irq: power-key-irq { > + samsung,pins = "gpx0-3"; > + samsung,pin-pud = <0>; Don't you want to set specific pin function? And what about drive strength? Best regards, Krzysztof > + }; > + > hdmi_hpd_irq: hdmi-hpd-irq { > samsung,pins = "gpx3-7"; > samsung,pin-function = <0>; > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 8:21 ` Krzysztof Kozlowski @ 2016-02-23 8:33 ` Krzysztof Kozlowski -1 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 8:33 UTC (permalink / raw) To: Anand Moon, Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel On 23.02.2016 17:21, Krzysztof Kozlowski wrote: > On 23.02.2016 17:01, Anand Moon wrote: >> Add support for gpio-based button on Odroid-XU3 boards >> for reboot/poweroff feature. >> >> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >> --- >> changes rebase based on linux next-20160222. >> >> Tested on Odroid-XU4 >> >> dmesg output. >> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >> --- >> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >> index 1bd507b..db9770b 100644 >> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >> @@ -11,6 +11,7 @@ >> */ >> >> #include <dt-bindings/clock/samsung,s2mps11.h> >> +#include <dt-bindings/input/input.h> >> #include <dt-bindings/interrupt-controller/irq.h> >> #include <dt-bindings/gpio/gpio.h> >> #include <dt-bindings/sound/samsung-i2s.h> >> @@ -54,6 +55,22 @@ >> #cooling-cells = <2>; >> cooling-levels = <0 130 170 230>; >> }; >> + >> + gpio_keys { >> + compatible = "gpio-keys"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&gpio_power_key>; >> + >> + power_key { >> + interrupt-parent = <&gpx0>; >> + interrupts = <3 IRQ_TYPE_NONE>; > > Hmmm.... why you specify the interrupts? > >> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; Please, explain it to me. The SW2 key is connected to PWRON of PMIC. However you are adding a GPIO key for external interrupt source 3 (XE.INT3)... which comes from PMIC's ONOB. It's interesting.... how does it work? The PMIC will generate ONOB interrupt on PWRON low->high change (when PWRHOLD is high)? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 8:33 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 8:33 UTC (permalink / raw) To: linux-arm-kernel On 23.02.2016 17:21, Krzysztof Kozlowski wrote: > On 23.02.2016 17:01, Anand Moon wrote: >> Add support for gpio-based button on Odroid-XU3 boards >> for reboot/poweroff feature. >> >> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >> --- >> changes rebase based on linux next-20160222. >> >> Tested on Odroid-XU4 >> >> dmesg output. >> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >> --- >> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >> index 1bd507b..db9770b 100644 >> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >> @@ -11,6 +11,7 @@ >> */ >> >> #include <dt-bindings/clock/samsung,s2mps11.h> >> +#include <dt-bindings/input/input.h> >> #include <dt-bindings/interrupt-controller/irq.h> >> #include <dt-bindings/gpio/gpio.h> >> #include <dt-bindings/sound/samsung-i2s.h> >> @@ -54,6 +55,22 @@ >> #cooling-cells = <2>; >> cooling-levels = <0 130 170 230>; >> }; >> + >> + gpio_keys { >> + compatible = "gpio-keys"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&gpio_power_key>; >> + >> + power_key { >> + interrupt-parent = <&gpx0>; >> + interrupts = <3 IRQ_TYPE_NONE>; > > Hmmm.... why you specify the interrupts? > >> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; Please, explain it to me. The SW2 key is connected to PWRON of PMIC. However you are adding a GPIO key for external interrupt source 3 (XE.INT3)... which comes from PMIC's ONOB. It's interesting.... how does it work? The PMIC will generate ONOB interrupt on PWRON low->high change (when PWRHOLD is high)? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 8:33 ` Krzysztof Kozlowski @ 2016-02-23 8:47 ` Krzysztof Kozlowski -1 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 8:47 UTC (permalink / raw) To: Anand Moon, Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel On 23.02.2016 17:33, Krzysztof Kozlowski wrote: > On 23.02.2016 17:21, Krzysztof Kozlowski wrote: >> On 23.02.2016 17:01, Anand Moon wrote: >>> Add support for gpio-based button on Odroid-XU3 boards >>> for reboot/poweroff feature. >>> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> changes rebase based on linux next-20160222. >>> >>> Tested on Odroid-XU4 >>> >>> dmesg output. >>> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >>> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >>> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >>> --- >>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> index 1bd507b..db9770b 100644 >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> @@ -11,6 +11,7 @@ >>> */ >>> >>> #include <dt-bindings/clock/samsung,s2mps11.h> >>> +#include <dt-bindings/input/input.h> >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/gpio/gpio.h> >>> #include <dt-bindings/sound/samsung-i2s.h> >>> @@ -54,6 +55,22 @@ >>> #cooling-cells = <2>; >>> cooling-levels = <0 130 170 230>; >>> }; >>> + >>> + gpio_keys { >>> + compatible = "gpio-keys"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&gpio_power_key>; >>> + >>> + power_key { >>> + interrupt-parent = <&gpx0>; >>> + interrupts = <3 IRQ_TYPE_NONE>; >> >> Hmmm.... why you specify the interrupts? >> >>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > > Please, explain it to me. The SW2 key is connected to PWRON of PMIC. > However you are adding a GPIO key for external interrupt source 3 > (XE.INT3)... which comes from PMIC's ONOB. > > It's interesting.... how does it work? The PMIC will generate ONOB > interrupt on PWRON low->high change (when PWRHOLD is high)? Hmmm... This is not well documented but apparently that is the case. The PMIC will generate ONOB interrupt (with 16 ms de-bounce for PWRON) on each PWRON flip. This looks kind of hacky or indirect usage. Using a GPIO key not for GPIO itself but for an interrupt generated by PMIC on a key press... Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 8:47 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 8:47 UTC (permalink / raw) To: linux-arm-kernel On 23.02.2016 17:33, Krzysztof Kozlowski wrote: > On 23.02.2016 17:21, Krzysztof Kozlowski wrote: >> On 23.02.2016 17:01, Anand Moon wrote: >>> Add support for gpio-based button on Odroid-XU3 boards >>> for reboot/poweroff feature. >>> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> changes rebase based on linux next-20160222. >>> >>> Tested on Odroid-XU4 >>> >>> dmesg output. >>> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >>> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >>> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >>> --- >>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> index 1bd507b..db9770b 100644 >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> @@ -11,6 +11,7 @@ >>> */ >>> >>> #include <dt-bindings/clock/samsung,s2mps11.h> >>> +#include <dt-bindings/input/input.h> >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/gpio/gpio.h> >>> #include <dt-bindings/sound/samsung-i2s.h> >>> @@ -54,6 +55,22 @@ >>> #cooling-cells = <2>; >>> cooling-levels = <0 130 170 230>; >>> }; >>> + >>> + gpio_keys { >>> + compatible = "gpio-keys"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&gpio_power_key>; >>> + >>> + power_key { >>> + interrupt-parent = <&gpx0>; >>> + interrupts = <3 IRQ_TYPE_NONE>; >> >> Hmmm.... why you specify the interrupts? >> >>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > > Please, explain it to me. The SW2 key is connected to PWRON of PMIC. > However you are adding a GPIO key for external interrupt source 3 > (XE.INT3)... which comes from PMIC's ONOB. > > It's interesting.... how does it work? The PMIC will generate ONOB > interrupt on PWRON low->high change (when PWRHOLD is high)? Hmmm... This is not well documented but apparently that is the case. The PMIC will generate ONOB interrupt (with 16 ms de-bounce for PWRON) on each PWRON flip. This looks kind of hacky or indirect usage. Using a GPIO key not for GPIO itself but for an interrupt generated by PMIC on a key press... Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 8:33 ` Krzysztof Kozlowski (?) @ 2016-02-23 9:17 ` Anand Moon -1 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 9:17 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel Hi Krzysztof, On 23 February 2016 at 14:03, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > On 23.02.2016 17:21, Krzysztof Kozlowski wrote: >> On 23.02.2016 17:01, Anand Moon wrote: >>> Add support for gpio-based button on Odroid-XU3 boards >>> for reboot/poweroff feature. >>> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> changes rebase based on linux next-20160222. >>> >>> Tested on Odroid-XU4 >>> >>> dmesg output. >>> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >>> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >>> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >>> --- >>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> index 1bd507b..db9770b 100644 >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> @@ -11,6 +11,7 @@ >>> */ >>> >>> #include <dt-bindings/clock/samsung,s2mps11.h> >>> +#include <dt-bindings/input/input.h> >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/gpio/gpio.h> >>> #include <dt-bindings/sound/samsung-i2s.h> >>> @@ -54,6 +55,22 @@ >>> #cooling-cells = <2>; >>> cooling-levels = <0 130 170 230>; >>> }; >>> + >>> + gpio_keys { >>> + compatible = "gpio-keys"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&gpio_power_key>; >>> + >>> + power_key { >>> + interrupt-parent = <&gpx0>; >>> + interrupts = <3 IRQ_TYPE_NONE>; >> >> Hmmm.... why you specify the interrupts? >> >>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > > Please, explain it to me. The SW2 key is connected to PWRON of PMIC. > However you are adding a GPIO key for external interrupt source 3 > (XE.INT3)... which comes from PMIC's ONOB. > > It's interesting.... how does it work? The PMIC will generate ONOB > interrupt on PWRON low->high change (when PWRHOLD is high)? > > Best regards, > Krzysztof > I have re-base my changes on HK dts. I could not find much details for the schematic diagram. I don't know much low level detains on this. How dose this works: This changes will initial the shudown/reboot on Ubuntu. I have also tested this similar feature on Odroid U3. Best Regards. -Anand Moon ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 9:17 ` Anand Moon 0 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 9:17 UTC (permalink / raw) To: linux-arm-kernel Hi Krzysztof, On 23 February 2016 at 14:03, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > On 23.02.2016 17:21, Krzysztof Kozlowski wrote: >> On 23.02.2016 17:01, Anand Moon wrote: >>> Add support for gpio-based button on Odroid-XU3 boards >>> for reboot/poweroff feature. >>> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> changes rebase based on linux next-20160222. >>> >>> Tested on Odroid-XU4 >>> >>> dmesg output. >>> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >>> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >>> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >>> --- >>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> index 1bd507b..db9770b 100644 >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> @@ -11,6 +11,7 @@ >>> */ >>> >>> #include <dt-bindings/clock/samsung,s2mps11.h> >>> +#include <dt-bindings/input/input.h> >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/gpio/gpio.h> >>> #include <dt-bindings/sound/samsung-i2s.h> >>> @@ -54,6 +55,22 @@ >>> #cooling-cells = <2>; >>> cooling-levels = <0 130 170 230>; >>> }; >>> + >>> + gpio_keys { >>> + compatible = "gpio-keys"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&gpio_power_key>; >>> + >>> + power_key { >>> + interrupt-parent = <&gpx0>; >>> + interrupts = <3 IRQ_TYPE_NONE>; >> >> Hmmm.... why you specify the interrupts? >> >>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > > Please, explain it to me. The SW2 key is connected to PWRON of PMIC. > However you are adding a GPIO key for external interrupt source 3 > (XE.INT3)... which comes from PMIC's ONOB. > > It's interesting.... how does it work? The PMIC will generate ONOB > interrupt on PWRON low->high change (when PWRHOLD is high)? > > Best regards, > Krzysztof > I have re-base my changes on HK dts. I could not find much details for the schematic diagram. I don't know much low level detains on this. How dose this works: This changes will initial the shudown/reboot on Ubuntu. I have also tested this similar feature on Odroid U3. Best Regards. -Anand Moon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 9:17 ` Anand Moon 0 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 9:17 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel Hi Krzysztof, On 23 February 2016 at 14:03, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > On 23.02.2016 17:21, Krzysztof Kozlowski wrote: >> On 23.02.2016 17:01, Anand Moon wrote: >>> Add support for gpio-based button on Odroid-XU3 boards >>> for reboot/poweroff feature. >>> >>> Signed-off-by: Anand Moon <linux.amoon@gmail.com> >>> --- >>> changes rebase based on linux next-20160222. >>> >>> Tested on Odroid-XU4 >>> >>> dmesg output. >>> [ 3.286068] of_get_named_gpiod_flags: parsed 'gpios' property of node '/gpio_keys/power_key[0]' - status (0) >>> [ 3.286206] gpio-11 (power key): gpiod_set_debounce: missing set() or set_debounce() operations >>> [ 3.286600] input: gpio_keys as /devices/platform/gpio_keys/input/input0 >>> --- >>> arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi | 22 ++++++++++++++++++++++ >>> 1 file changed, 22 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> index 1bd507b..db9770b 100644 >>> --- a/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu3-common.dtsi >>> @@ -11,6 +11,7 @@ >>> */ >>> >>> #include <dt-bindings/clock/samsung,s2mps11.h> >>> +#include <dt-bindings/input/input.h> >>> #include <dt-bindings/interrupt-controller/irq.h> >>> #include <dt-bindings/gpio/gpio.h> >>> #include <dt-bindings/sound/samsung-i2s.h> >>> @@ -54,6 +55,22 @@ >>> #cooling-cells = <2>; >>> cooling-levels = <0 130 170 230>; >>> }; >>> + >>> + gpio_keys { >>> + compatible = "gpio-keys"; >>> + pinctrl-names = "default"; >>> + pinctrl-0 = <&gpio_power_key>; >>> + >>> + power_key { >>> + interrupt-parent = <&gpx0>; >>> + interrupts = <3 IRQ_TYPE_NONE>; >> >> Hmmm.... why you specify the interrupts? >> >>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; > > Please, explain it to me. The SW2 key is connected to PWRON of PMIC. > However you are adding a GPIO key for external interrupt source 3 > (XE.INT3)... which comes from PMIC's ONOB. > > It's interesting.... how does it work? The PMIC will generate ONOB > interrupt on PWRON low->high change (when PWRHOLD is high)? > > Best regards, > Krzysztof > I have re-base my changes on HK dts. I could not find much details for the schematic diagram. I don't know much low level detains on this. How dose this works: This changes will initial the shudown/reboot on Ubuntu. I have also tested this similar feature on Odroid U3. Best Regards. -Anand Moon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 9:17 ` Anand Moon (?) @ 2016-02-23 12:02 ` Krzysztof Kozlowski -1 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 12:02 UTC (permalink / raw) To: Anand Moon Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: > Hi Krzysztof, > > On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>> }; >>>> + >>>> + gpio_keys { >>>> + compatible = "gpio-keys"; >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&gpio_power_key>; >>>> + >>>> + power_key { >>>> + interrupt-parent = <&gpx0>; >>>> + interrupts = <3 IRQ_TYPE_NONE>; >>> >>> Hmmm.... why you specify the interrupts? >>> >>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >> >> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >> However you are adding a GPIO key for external interrupt source 3 >> (XE.INT3)... which comes from PMIC's ONOB. >> >> It's interesting.... how does it work? The PMIC will generate ONOB >> interrupt on PWRON low->high change (when PWRHOLD is high)? >> >> Best regards, >> Krzysztof >> > > I have re-base my changes on HK dts. > I could not find much details for the schematic diagram. > I don't know much low level detains on this. If I understand this correctly: you just took some vendor code, similar to existing code of Odroid U3, and without full understanding of the code nor checking with public schematics, you sent it. Taking vendor stuff is okay but you need to think about it, use it as an example and deliver proper solution based on that. Not copy-paste. > How dose this works: This changes will initial the shudown/reboot on Ubuntu. > I have also tested this similar feature on Odroid U3. Great :), yes it works because ONOB interrupt from PMIC is generated on key press. However this is not a strictly speaking key... I don't really know what to do with this commit and your explanation (Hardkernel has such code) is not sufficient to convince me. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 12:02 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 12:02 UTC (permalink / raw) To: linux-arm-kernel 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: > Hi Krzysztof, > > On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>> }; >>>> + >>>> + gpio_keys { >>>> + compatible = "gpio-keys"; >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&gpio_power_key>; >>>> + >>>> + power_key { >>>> + interrupt-parent = <&gpx0>; >>>> + interrupts = <3 IRQ_TYPE_NONE>; >>> >>> Hmmm.... why you specify the interrupts? >>> >>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >> >> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >> However you are adding a GPIO key for external interrupt source 3 >> (XE.INT3)... which comes from PMIC's ONOB. >> >> It's interesting.... how does it work? The PMIC will generate ONOB >> interrupt on PWRON low->high change (when PWRHOLD is high)? >> >> Best regards, >> Krzysztof >> > > I have re-base my changes on HK dts. > I could not find much details for the schematic diagram. > I don't know much low level detains on this. If I understand this correctly: you just took some vendor code, similar to existing code of Odroid U3, and without full understanding of the code nor checking with public schematics, you sent it. Taking vendor stuff is okay but you need to think about it, use it as an example and deliver proper solution based on that. Not copy-paste. > How dose this works: This changes will initial the shudown/reboot on Ubuntu. > I have also tested this similar feature on Odroid U3. Great :), yes it works because ONOB interrupt from PMIC is generated on key press. However this is not a strictly speaking key... I don't really know what to do with this commit and your explanation (Hardkernel has such code) is not sufficient to convince me. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 12:02 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-23 12:02 UTC (permalink / raw) To: Anand Moon Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: > Hi Krzysztof, > > On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>> }; >>>> + >>>> + gpio_keys { >>>> + compatible = "gpio-keys"; >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&gpio_power_key>; >>>> + >>>> + power_key { >>>> + interrupt-parent = <&gpx0>; >>>> + interrupts = <3 IRQ_TYPE_NONE>; >>> >>> Hmmm.... why you specify the interrupts? >>> >>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >> >> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >> However you are adding a GPIO key for external interrupt source 3 >> (XE.INT3)... which comes from PMIC's ONOB. >> >> It's interesting.... how does it work? The PMIC will generate ONOB >> interrupt on PWRON low->high change (when PWRHOLD is high)? >> >> Best regards, >> Krzysztof >> > > I have re-base my changes on HK dts. > I could not find much details for the schematic diagram. > I don't know much low level detains on this. If I understand this correctly: you just took some vendor code, similar to existing code of Odroid U3, and without full understanding of the code nor checking with public schematics, you sent it. Taking vendor stuff is okay but you need to think about it, use it as an example and deliver proper solution based on that. Not copy-paste. > How dose this works: This changes will initial the shudown/reboot on Ubuntu. > I have also tested this similar feature on Odroid U3. Great :), yes it works because ONOB interrupt from PMIC is generated on key press. However this is not a strictly speaking key... I don't really know what to do with this commit and your explanation (Hardkernel has such code) is not sufficient to convince me. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 12:02 ` Krzysztof Kozlowski (?) @ 2016-02-23 14:16 ` Anand Moon -1 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 14:16 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel Hi Krzysztof, On 23 February 2016 at 17:32, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: >> Hi Krzysztof, >> >> On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>>> }; >>>>> + >>>>> + gpio_keys { >>>>> + compatible = "gpio-keys"; >>>>> + pinctrl-names = "default"; >>>>> + pinctrl-0 = <&gpio_power_key>; >>>>> + >>>>> + power_key { >>>>> + interrupt-parent = <&gpx0>; >>>>> + interrupts = <3 IRQ_TYPE_NONE>; >>>> >>>> Hmmm.... why you specify the interrupts? >>>> >>>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >>> >>> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >>> However you are adding a GPIO key for external interrupt source 3 >>> (XE.INT3)... which comes from PMIC's ONOB. >>> >>> It's interesting.... how does it work? The PMIC will generate ONOB >>> interrupt on PWRON low->high change (when PWRHOLD is high)? >>> >>> Best regards, >>> Krzysztof >>> >> >> I have re-base my changes on HK dts. >> I could not find much details for the schematic diagram. >> I don't know much low level detains on this. > > If I understand this correctly: you just took some vendor code, > similar to existing code of Odroid U3, and without full understanding > of the code nor checking with public schematics, you sent it. > > Taking vendor stuff is okay but you need to think about it, use it as > an example and deliver proper solution based on that. Not copy-paste. > >> How dose this works: This changes will initial the shudown/reboot on Ubuntu. >> I have also tested this similar feature on Odroid U3. > > Great :), yes it works because ONOB interrupt from PMIC is generated > on key press. However this is not a strictly speaking key... I don't > really know what to do with this commit and your explanation > (Hardkernel has such code) is not sufficient to convince me. > > Best regards, > Krzysztof Nothing come easy to me, I had to do bit of work on this. Internally I might not know the detail of the board and features of the registers of the PMIC. I have to improvise some time for changes. I don't know who is the original author of the code. It's relay hard to convince you on the changes, as I continue to repeat the mistake. I will try to improve my commits a be specific to the changes in the future. Best Regards, -Anand Moon ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 14:16 ` Anand Moon 0 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 14:16 UTC (permalink / raw) To: linux-arm-kernel Hi Krzysztof, On 23 February 2016 at 17:32, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: >> Hi Krzysztof, >> >> On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>>> }; >>>>> + >>>>> + gpio_keys { >>>>> + compatible = "gpio-keys"; >>>>> + pinctrl-names = "default"; >>>>> + pinctrl-0 = <&gpio_power_key>; >>>>> + >>>>> + power_key { >>>>> + interrupt-parent = <&gpx0>; >>>>> + interrupts = <3 IRQ_TYPE_NONE>; >>>> >>>> Hmmm.... why you specify the interrupts? >>>> >>>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >>> >>> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >>> However you are adding a GPIO key for external interrupt source 3 >>> (XE.INT3)... which comes from PMIC's ONOB. >>> >>> It's interesting.... how does it work? The PMIC will generate ONOB >>> interrupt on PWRON low->high change (when PWRHOLD is high)? >>> >>> Best regards, >>> Krzysztof >>> >> >> I have re-base my changes on HK dts. >> I could not find much details for the schematic diagram. >> I don't know much low level detains on this. > > If I understand this correctly: you just took some vendor code, > similar to existing code of Odroid U3, and without full understanding > of the code nor checking with public schematics, you sent it. > > Taking vendor stuff is okay but you need to think about it, use it as > an example and deliver proper solution based on that. Not copy-paste. > >> How dose this works: This changes will initial the shudown/reboot on Ubuntu. >> I have also tested this similar feature on Odroid U3. > > Great :), yes it works because ONOB interrupt from PMIC is generated > on key press. However this is not a strictly speaking key... I don't > really know what to do with this commit and your explanation > (Hardkernel has such code) is not sufficient to convince me. > > Best regards, > Krzysztof Nothing come easy to me, I had to do bit of work on this. Internally I might not know the detail of the board and features of the registers of the PMIC. I have to improvise some time for changes. I don't know who is the original author of the code. It's relay hard to convince you on the changes, as I continue to repeat the mistake. I will try to improve my commits a be specific to the changes in the future. Best Regards, -Anand Moon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-23 14:16 ` Anand Moon 0 siblings, 0 replies; 20+ messages in thread From: Anand Moon @ 2016-02-23 14:16 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel Hi Krzysztof, On 23 February 2016 at 17:32, Krzysztof Kozlowski <k.kozlowski@samsung.com> wrote: > 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: >> Hi Krzysztof, >> >> On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>>> }; >>>>> + >>>>> + gpio_keys { >>>>> + compatible = "gpio-keys"; >>>>> + pinctrl-names = "default"; >>>>> + pinctrl-0 = <&gpio_power_key>; >>>>> + >>>>> + power_key { >>>>> + interrupt-parent = <&gpx0>; >>>>> + interrupts = <3 IRQ_TYPE_NONE>; >>>> >>>> Hmmm.... why you specify the interrupts? >>>> >>>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >>> >>> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >>> However you are adding a GPIO key for external interrupt source 3 >>> (XE.INT3)... which comes from PMIC's ONOB. >>> >>> It's interesting.... how does it work? The PMIC will generate ONOB >>> interrupt on PWRON low->high change (when PWRHOLD is high)? >>> >>> Best regards, >>> Krzysztof >>> >> >> I have re-base my changes on HK dts. >> I could not find much details for the schematic diagram. >> I don't know much low level detains on this. > > If I understand this correctly: you just took some vendor code, > similar to existing code of Odroid U3, and without full understanding > of the code nor checking with public schematics, you sent it. > > Taking vendor stuff is okay but you need to think about it, use it as > an example and deliver proper solution based on that. Not copy-paste. > >> How dose this works: This changes will initial the shudown/reboot on Ubuntu. >> I have also tested this similar feature on Odroid U3. > > Great :), yes it works because ONOB interrupt from PMIC is generated > on key press. However this is not a strictly speaking key... I don't > really know what to do with this commit and your explanation > (Hardkernel has such code) is not sufficient to convince me. > > Best regards, > Krzysztof Nothing come easy to me, I had to do bit of work on this. Internally I might not know the detail of the board and features of the registers of the PMIC. I have to improvise some time for changes. I don't know who is the original author of the code. It's relay hard to convince you on the changes, as I continue to repeat the mistake. I will try to improve my commits a be specific to the changes in the future. Best Regards, -Anand Moon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 2016-02-23 14:16 ` Anand Moon (?) @ 2016-02-24 7:27 ` Krzysztof Kozlowski -1 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-24 7:27 UTC (permalink / raw) To: Anand Moon Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel On 23.02.2016 23:16, Anand Moon wrote: > Hi Krzysztof, > > On 23 February 2016 at 17:32, Krzysztof Kozlowski > <k.kozlowski@samsung.com> wrote: >> 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: >>> Hi Krzysztof, >>> >>> On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>>>> }; >>>>>> + >>>>>> + gpio_keys { >>>>>> + compatible = "gpio-keys"; >>>>>> + pinctrl-names = "default"; >>>>>> + pinctrl-0 = <&gpio_power_key>; >>>>>> + >>>>>> + power_key { >>>>>> + interrupt-parent = <&gpx0>; >>>>>> + interrupts = <3 IRQ_TYPE_NONE>; >>>>> >>>>> Hmmm.... why you specify the interrupts? >>>>> >>>>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >>>> >>>> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >>>> However you are adding a GPIO key for external interrupt source 3 >>>> (XE.INT3)... which comes from PMIC's ONOB. >>>> >>>> It's interesting.... how does it work? The PMIC will generate ONOB >>>> interrupt on PWRON low->high change (when PWRHOLD is high)? >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >>> I have re-base my changes on HK dts. >>> I could not find much details for the schematic diagram. >>> I don't know much low level detains on this. >> >> If I understand this correctly: you just took some vendor code, >> similar to existing code of Odroid U3, and without full understanding >> of the code nor checking with public schematics, you sent it. >> >> Taking vendor stuff is okay but you need to think about it, use it as >> an example and deliver proper solution based on that. Not copy-paste. >> >>> How dose this works: This changes will initial the shudown/reboot on Ubuntu. >>> I have also tested this similar feature on Odroid U3. >> >> Great :), yes it works because ONOB interrupt from PMIC is generated >> on key press. However this is not a strictly speaking key... I don't >> really know what to do with this commit and your explanation >> (Hardkernel has such code) is not sufficient to convince me. >> >> Best regards, >> Krzysztof > > Nothing come easy to me, I had to do bit of work on this. > Internally I might not know the detail of the board and features of > the registers of the PMIC. > I have to improvise some time for changes. I don't know who is the > original author of the code. > > It's relay hard to convince you on the changes, as I continue to > repeat the mistake. > I will try to improve my commits a be specific to the changes in the future. No worries. :) Anand, Anyway I was thinking about this and discussed it even offline. This is a little bit funny design choice because: 1. The generated interrupt (on line ONOB) is not coming from the physical key. It is coming from the PMIC, after receiving the change of PWRON level (which comes from the key). 2. The behaviour and correlation between PWRON and ONOB is poorly described in datasheet. Only one place mentions it. 3. The other funny idea we had, was to add a input driver for PMIC... but that IMHO won't solve anything. It won't even make this logically correct. 4. What I don't like personally, that I had to discover all of it, instead of reading this in commit message coming from you. I would like to see the explanation of this design (ONOB, PWRON etc) in comment in DTS. Written with your own words, not mine (which would be a confirmation that you understood the code you wrote and sent). Why? Because you are adding a gpio-key for something which is not a key. Funny, isn't it? :) 5. If anyone else has some ideas how to solve this (a key-PMIC mixture), please share! Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-24 7:27 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-24 7:27 UTC (permalink / raw) To: linux-arm-kernel On 23.02.2016 23:16, Anand Moon wrote: > Hi Krzysztof, > > On 23 February 2016 at 17:32, Krzysztof Kozlowski > <k.kozlowski@samsung.com> wrote: >> 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: >>> Hi Krzysztof, >>> >>> On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>>>> }; >>>>>> + >>>>>> + gpio_keys { >>>>>> + compatible = "gpio-keys"; >>>>>> + pinctrl-names = "default"; >>>>>> + pinctrl-0 = <&gpio_power_key>; >>>>>> + >>>>>> + power_key { >>>>>> + interrupt-parent = <&gpx0>; >>>>>> + interrupts = <3 IRQ_TYPE_NONE>; >>>>> >>>>> Hmmm.... why you specify the interrupts? >>>>> >>>>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >>>> >>>> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >>>> However you are adding a GPIO key for external interrupt source 3 >>>> (XE.INT3)... which comes from PMIC's ONOB. >>>> >>>> It's interesting.... how does it work? The PMIC will generate ONOB >>>> interrupt on PWRON low->high change (when PWRHOLD is high)? >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >>> I have re-base my changes on HK dts. >>> I could not find much details for the schematic diagram. >>> I don't know much low level detains on this. >> >> If I understand this correctly: you just took some vendor code, >> similar to existing code of Odroid U3, and without full understanding >> of the code nor checking with public schematics, you sent it. >> >> Taking vendor stuff is okay but you need to think about it, use it as >> an example and deliver proper solution based on that. Not copy-paste. >> >>> How dose this works: This changes will initial the shudown/reboot on Ubuntu. >>> I have also tested this similar feature on Odroid U3. >> >> Great :), yes it works because ONOB interrupt from PMIC is generated >> on key press. However this is not a strictly speaking key... I don't >> really know what to do with this commit and your explanation >> (Hardkernel has such code) is not sufficient to convince me. >> >> Best regards, >> Krzysztof > > Nothing come easy to me, I had to do bit of work on this. > Internally I might not know the detail of the board and features of > the registers of the PMIC. > I have to improvise some time for changes. I don't know who is the > original author of the code. > > It's relay hard to convince you on the changes, as I continue to > repeat the mistake. > I will try to improve my commits a be specific to the changes in the future. No worries. :) Anand, Anyway I was thinking about this and discussed it even offline. This is a little bit funny design choice because: 1. The generated interrupt (on line ONOB) is not coming from the physical key. It is coming from the PMIC, after receiving the change of PWRON level (which comes from the key). 2. The behaviour and correlation between PWRON and ONOB is poorly described in datasheet. Only one place mentions it. 3. The other funny idea we had, was to add a input driver for PMIC... but that IMHO won't solve anything. It won't even make this logically correct. 4. What I don't like personally, that I had to discover all of it, instead of reading this in commit message coming from you. I would like to see the explanation of this design (ONOB, PWRON etc) in comment in DTS. Written with your own words, not mine (which would be a confirmation that you understood the code you wrote and sent). Why? Because you are adding a gpio-key for something which is not a key. Funny, isn't it? :) 5. If anyone else has some ideas how to solve this (a key-PMIC mixture), please share! Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 @ 2016-02-24 7:27 ` Krzysztof Kozlowski 0 siblings, 0 replies; 20+ messages in thread From: Krzysztof Kozlowski @ 2016-02-24 7:27 UTC (permalink / raw) To: Anand Moon Cc: Kukjin Kim, Javier Martinez Canillas, Marek Szyprowski, devicetree, linux-arm-kernel, linux-samsung-soc, Linux Kernel On 23.02.2016 23:16, Anand Moon wrote: > Hi Krzysztof, > > On 23 February 2016 at 17:32, Krzysztof Kozlowski > <k.kozlowski@samsung.com> wrote: >> 2016-02-23 18:17 GMT+09:00 Anand Moon <linux.amoon@gmail.com>: >>> Hi Krzysztof, >>> >>> On 23 February 2016 at 14:03, Krzysztof Kozlowski >>>>>> }; >>>>>> + >>>>>> + gpio_keys { >>>>>> + compatible = "gpio-keys"; >>>>>> + pinctrl-names = "default"; >>>>>> + pinctrl-0 = <&gpio_power_key>; >>>>>> + >>>>>> + power_key { >>>>>> + interrupt-parent = <&gpx0>; >>>>>> + interrupts = <3 IRQ_TYPE_NONE>; >>>>> >>>>> Hmmm.... why you specify the interrupts? >>>>> >>>>>> + gpios = <&gpx0 3 GPIO_ACTIVE_LOW>; >>>> >>>> Please, explain it to me. The SW2 key is connected to PWRON of PMIC. >>>> However you are adding a GPIO key for external interrupt source 3 >>>> (XE.INT3)... which comes from PMIC's ONOB. >>>> >>>> It's interesting.... how does it work? The PMIC will generate ONOB >>>> interrupt on PWRON low->high change (when PWRHOLD is high)? >>>> >>>> Best regards, >>>> Krzysztof >>>> >>> >>> I have re-base my changes on HK dts. >>> I could not find much details for the schematic diagram. >>> I don't know much low level detains on this. >> >> If I understand this correctly: you just took some vendor code, >> similar to existing code of Odroid U3, and without full understanding >> of the code nor checking with public schematics, you sent it. >> >> Taking vendor stuff is okay but you need to think about it, use it as >> an example and deliver proper solution based on that. Not copy-paste. >> >>> How dose this works: This changes will initial the shudown/reboot on Ubuntu. >>> I have also tested this similar feature on Odroid U3. >> >> Great :), yes it works because ONOB interrupt from PMIC is generated >> on key press. However this is not a strictly speaking key... I don't >> really know what to do with this commit and your explanation >> (Hardkernel has such code) is not sufficient to convince me. >> >> Best regards, >> Krzysztof > > Nothing come easy to me, I had to do bit of work on this. > Internally I might not know the detail of the board and features of > the registers of the PMIC. > I have to improvise some time for changes. I don't know who is the > original author of the code. > > It's relay hard to convince you on the changes, as I continue to > repeat the mistake. > I will try to improve my commits a be specific to the changes in the future. No worries. :) Anand, Anyway I was thinking about this and discussed it even offline. This is a little bit funny design choice because: 1. The generated interrupt (on line ONOB) is not coming from the physical key. It is coming from the PMIC, after receiving the change of PWRON level (which comes from the key). 2. The behaviour and correlation between PWRON and ONOB is poorly described in datasheet. Only one place mentions it. 3. The other funny idea we had, was to add a input driver for PMIC... but that IMHO won't solve anything. It won't even make this logically correct. 4. What I don't like personally, that I had to discover all of it, instead of reading this in commit message coming from you. I would like to see the explanation of this design (ONOB, PWRON etc) in comment in DTS. Written with your own words, not mine (which would be a confirmation that you understood the code you wrote and sent). Why? Because you are adding a gpio-key for something which is not a key. Funny, isn't it? :) 5. If anyone else has some ideas how to solve this (a key-PMIC mixture), please share! Best regards, Krzysztof ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-02-24 7:28 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-02-23 8:01 [PATCH] ARM: dts: add support for gpio buttons for exynos5422-odroidxu3 Anand Moon 2016-02-23 8:01 ` Anand Moon 2016-02-23 8:21 ` Krzysztof Kozlowski 2016-02-23 8:21 ` Krzysztof Kozlowski 2016-02-23 8:33 ` Krzysztof Kozlowski 2016-02-23 8:33 ` Krzysztof Kozlowski 2016-02-23 8:47 ` Krzysztof Kozlowski 2016-02-23 8:47 ` Krzysztof Kozlowski 2016-02-23 9:17 ` Anand Moon 2016-02-23 9:17 ` Anand Moon 2016-02-23 9:17 ` Anand Moon 2016-02-23 12:02 ` Krzysztof Kozlowski 2016-02-23 12:02 ` Krzysztof Kozlowski 2016-02-23 12:02 ` Krzysztof Kozlowski 2016-02-23 14:16 ` Anand Moon 2016-02-23 14:16 ` Anand Moon 2016-02-23 14:16 ` Anand Moon 2016-02-24 7:27 ` Krzysztof Kozlowski 2016-02-24 7:27 ` Krzysztof Kozlowski 2016-02-24 7:27 ` Krzysztof Kozlowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.