* [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma @ 2024-09-25 7:28 Daniel Semkowicz 2024-09-26 12:34 ` Quentin Schulz 0 siblings, 1 reply; 6+ messages in thread From: Daniel Semkowicz @ 2024-09-25 7:28 UTC (permalink / raw) To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner Cc: Daniel Semkowicz, Dragan Simic, Farouk Bouabid, Iskander Amara, Quentin Schulz, Vahe Grigoryan, devicetree, linux-arm-kernel, linux-kernel, linux-rockchip There is a PWRBTN# input pin exposed on a Q7 connector. The pin is routed to a GPIO0_A1 through a diode. Q7 specification describes the PWRBTN# pin as a Power Button signal. Configure the pin as KEY_POWER, so it can function as power button and trigger device shutdown. Add the pin definition to RK3399 Puma dts, so it can be reused by derived platforms, but keep it disabled by default. Enable the power button input on Haikou development board. Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> --- .../boot/dts/rockchip/rk3399-puma-haikou.dts | 4 ++++ arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts index f6f15946579e..0999026b16d0 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts @@ -143,6 +143,10 @@ vddd_codec: vddd-codec { }; }; +&gpio_key_power { + status = "okay"; +}; + &hdmi { ddc-i2c-bus = <&i2c3>; status = "okay"; diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi index 650b1ba9c192..389ffe604e74 100644 --- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi @@ -3,6 +3,7 @@ * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH */ +#include <dt-bindings/input/input.h> #include <dt-bindings/pwm/pwm.h> #include "rk3399.dtsi" @@ -39,6 +40,19 @@ clkin_gmac: external-gmac-clock { #clock-cells = <0>; }; + gpio_key_power: gpio-key-power { + compatible = "gpio-keys"; + pinctrl-0 = <&pwrbtn_pin>; + pinctrl-names = "default"; + status = "disabled"; + + button-pwrbtn-n { + gpios = <&gpio0 RK_PA1 GPIO_ACTIVE_LOW>; + label = "PWRBTN#"; + linux,code = <KEY_POWER>; + }; + }; + vcc1v2_phy: vcc1v2-phy { compatible = "regulator-fixed"; regulator-name = "vcc1v2_phy"; @@ -475,6 +489,14 @@ &pinctrl { pinctrl-names = "default"; pinctrl-0 = <&q7_thermal_pin &bios_disable_override_hog_pin>; + buttons { + pwrbtn_pin: pwrbtn-pin { + rockchip,pins = + /* PWRBTN# */ + <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_up>; + }; + }; + gpios { bios_disable_override_hog_pin: bios-disable-override-hog-pin { rockchip,pins = -- 2.46.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma 2024-09-25 7:28 [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma Daniel Semkowicz @ 2024-09-26 12:34 ` Quentin Schulz 2024-09-30 8:49 ` Heiko Stübner 0 siblings, 1 reply; 6+ messages in thread From: Quentin Schulz @ 2024-09-26 12:34 UTC (permalink / raw) To: Daniel Semkowicz, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner Cc: Dragan Simic, Farouk Bouabid, Quentin Schulz, Vahe Grigoryan, devicetree, linux-arm-kernel, linux-kernel, linux-rockchip Hi Daniel, On 9/25/24 9:28 AM, Daniel Semkowicz wrote: > There is a PWRBTN# input pin exposed on a Q7 connector. The pin > is routed to a GPIO0_A1 through a diode. Q7 specification describes > the PWRBTN# pin as a Power Button signal. > Configure the pin as KEY_POWER, so it can function as power button and > trigger device shutdown. > Add the pin definition to RK3399 Puma dts, so it can be reused > by derived platforms, but keep it disabled by default. > > Enable the power button input on Haikou development board. > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> This works, thanks. Tested-by: Quentin Schulz <quentin.schulz@cherry.de> Now I have some questions I wasn't able to answer myself, maybe someone can provide some feedback on those :) We already have a gpio-keys for buttons on Haikou, c.f. https://elixir.bootlin.com/linux/v6.11/source/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#L22. Those signals are directly routed to the SoM and follow the Qseven standard. The same applies to PWRBTN# signal. However, here we have one gpio-keys for PWRBTN# in Puma DTSI and one gpio-keys for the buttons and sliders on Haikou devkit in Haikou DTS. I'm a bit undecided on where this should go. Having all button/slider signals following the Qseven standard in Puma DTSI and enable the gpio-keys only in the devkit would make sense to me, so that other baseboards could easily make use of it. However, things get complicated if the baseboard manufacturer decides to only implement **some** of the signals, for which we then need to remove some nodes from gpio-keys (and pinctrl entries) since gpio-keys doesn't check the "status" property in its child nodes (though that could be fixed). At which point, it's not entirely clear if having it in Puma DTSI is actually beneficial. Someone has an opinion/recommendation on that? Cheers, Quentin > --- > > .../boot/dts/rockchip/rk3399-puma-haikou.dts | 4 ++++ > arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 22 +++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts > index f6f15946579e..0999026b16d0 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts > @@ -143,6 +143,10 @@ vddd_codec: vddd-codec { > }; > }; > > +&gpio_key_power { > + status = "okay"; > +}; > + > &hdmi { > ddc-i2c-bus = <&i2c3>; > status = "okay"; > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi > index 650b1ba9c192..389ffe604e74 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi > +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi > @@ -3,6 +3,7 @@ > * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH > */ > > +#include <dt-bindings/input/input.h> > #include <dt-bindings/pwm/pwm.h> > #include "rk3399.dtsi" > > @@ -39,6 +40,19 @@ clkin_gmac: external-gmac-clock { > #clock-cells = <0>; > }; > > + gpio_key_power: gpio-key-power { > + compatible = "gpio-keys"; > + pinctrl-0 = <&pwrbtn_pin>; > + pinctrl-names = "default"; > + status = "disabled"; > + > + button-pwrbtn-n { > + gpios = <&gpio0 RK_PA1 GPIO_ACTIVE_LOW>; > + label = "PWRBTN#"; > + linux,code = <KEY_POWER>; > + }; > + }; > + > vcc1v2_phy: vcc1v2-phy { > compatible = "regulator-fixed"; > regulator-name = "vcc1v2_phy"; > @@ -475,6 +489,14 @@ &pinctrl { > pinctrl-names = "default"; > pinctrl-0 = <&q7_thermal_pin &bios_disable_override_hog_pin>; > > + buttons { > + pwrbtn_pin: pwrbtn-pin { > + rockchip,pins = > + /* PWRBTN# */ > + <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_up>; > + }; > + }; > + > gpios { > bios_disable_override_hog_pin: bios-disable-override-hog-pin { > rockchip,pins = ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma 2024-09-26 12:34 ` Quentin Schulz @ 2024-09-30 8:49 ` Heiko Stübner 2024-09-30 9:11 ` Quentin Schulz 0 siblings, 1 reply; 6+ messages in thread From: Heiko Stübner @ 2024-09-30 8:49 UTC (permalink / raw) To: Daniel Semkowicz, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Quentin Schulz Cc: Dragan Simic, Farouk Bouabid, Quentin Schulz, Vahe Grigoryan, devicetree, linux-arm-kernel, linux-kernel, linux-rockchip Hey Quentin, Daniel, Am Donnerstag, 26. September 2024, 14:34:30 CEST schrieb Quentin Schulz: > On 9/25/24 9:28 AM, Daniel Semkowicz wrote: > > There is a PWRBTN# input pin exposed on a Q7 connector. The pin > > is routed to a GPIO0_A1 through a diode. Q7 specification describes > > the PWRBTN# pin as a Power Button signal. > > Configure the pin as KEY_POWER, so it can function as power button and > > trigger device shutdown. > > Add the pin definition to RK3399 Puma dts, so it can be reused > > by derived platforms, but keep it disabled by default. > > > > Enable the power button input on Haikou development board. > > > > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > > This works, thanks. > > Tested-by: Quentin Schulz <quentin.schulz@cherry.de> > > Now I have some questions I wasn't able to answer myself, maybe someone > can provide some feedback on those :) > > We already have a gpio-keys for buttons on Haikou, c.f. > https://elixir.bootlin.com/linux/v6.11/source/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#L22. > Those signals are directly routed to the SoM and follow the Qseven standard. > > The same applies to PWRBTN# signal. > > However, here we have one gpio-keys for PWRBTN# in Puma DTSI and one > gpio-keys for the buttons and sliders on Haikou devkit in Haikou DTS. > > I'm a bit undecided on where this should go. > > Having all button/slider signals following the Qseven standard in Puma > DTSI and enable the gpio-keys only in the devkit would make sense to me, > so that other baseboards could easily make use of it. However, things > get complicated if the baseboard manufacturer decides to only implement > **some** of the signals, for which we then need to remove some nodes > from gpio-keys (and pinctrl entries) since gpio-keys doesn't check the > "status" property in its child nodes (though that could be fixed). At > which point, it's not entirely clear if having it in Puma DTSI is > actually beneficial. > > Someone has an opinion/recommendation on that? I guess from a platform perspective nobody really cares, so as that is "your" board, it comes down to a policy decision on your part ;-) . While pins follow the q7 standard, there may very well be some lax handling of that standard in some places, and I guess gpio lines could be re-used for something else if needed, as something like the lid-switch is probably non-essential. Also a gpio-key input does not create that much code-overhead if replicated, so personally I'd just stick the power-button with the other buttons in the haikou dts. Which is also a way better thing than having multiple gpio-keys instances that userspace then has to handle. Heiko > > --- > > > > .../boot/dts/rockchip/rk3399-puma-haikou.dts | 4 ++++ > > arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi | 22 +++++++++++++++++++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts > > index f6f15946579e..0999026b16d0 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts > > @@ -143,6 +143,10 @@ vddd_codec: vddd-codec { > > }; > > }; > > > > +&gpio_key_power { > > + status = "okay"; > > +}; > > + > > &hdmi { > > ddc-i2c-bus = <&i2c3>; > > status = "okay"; > > diff --git a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi > > index 650b1ba9c192..389ffe604e74 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi > > +++ b/arch/arm64/boot/dts/rockchip/rk3399-puma.dtsi > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2017 Theobroma Systems Design und Consulting GmbH > > */ > > > > +#include <dt-bindings/input/input.h> > > #include <dt-bindings/pwm/pwm.h> > > #include "rk3399.dtsi" > > > > @@ -39,6 +40,19 @@ clkin_gmac: external-gmac-clock { > > #clock-cells = <0>; > > }; > > > > + gpio_key_power: gpio-key-power { > > + compatible = "gpio-keys"; > > + pinctrl-0 = <&pwrbtn_pin>; > > + pinctrl-names = "default"; > > + status = "disabled"; > > + > > + button-pwrbtn-n { > > + gpios = <&gpio0 RK_PA1 GPIO_ACTIVE_LOW>; > > + label = "PWRBTN#"; > > + linux,code = <KEY_POWER>; > > + }; > > + }; > > + > > vcc1v2_phy: vcc1v2-phy { > > compatible = "regulator-fixed"; > > regulator-name = "vcc1v2_phy"; > > @@ -475,6 +489,14 @@ &pinctrl { > > pinctrl-names = "default"; > > pinctrl-0 = <&q7_thermal_pin &bios_disable_override_hog_pin>; > > > > + buttons { > > + pwrbtn_pin: pwrbtn-pin { > > + rockchip,pins = > > + /* PWRBTN# */ > > + <0 RK_PA1 RK_FUNC_GPIO &pcfg_pull_up>; > > + }; > > + }; > > + > > gpios { > > bios_disable_override_hog_pin: bios-disable-override-hog-pin { > > rockchip,pins = > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma 2024-09-30 8:49 ` Heiko Stübner @ 2024-09-30 9:11 ` Quentin Schulz 2024-09-30 9:21 ` Heiko Stübner 0 siblings, 1 reply; 6+ messages in thread From: Quentin Schulz @ 2024-09-30 9:11 UTC (permalink / raw) To: Heiko Stübner, Daniel Semkowicz, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: Dragan Simic, Farouk Bouabid, Quentin Schulz, Vahe Grigoryan, devicetree, linux-arm-kernel, linux-kernel, linux-rockchip Hi Heiko, On 9/30/24 10:49 AM, Heiko Stübner wrote: > Hey Quentin, Daniel, > > Am Donnerstag, 26. September 2024, 14:34:30 CEST schrieb Quentin Schulz: >> On 9/25/24 9:28 AM, Daniel Semkowicz wrote: >>> There is a PWRBTN# input pin exposed on a Q7 connector. The pin >>> is routed to a GPIO0_A1 through a diode. Q7 specification describes >>> the PWRBTN# pin as a Power Button signal. >>> Configure the pin as KEY_POWER, so it can function as power button and >>> trigger device shutdown. >>> Add the pin definition to RK3399 Puma dts, so it can be reused >>> by derived platforms, but keep it disabled by default. >>> >>> Enable the power button input on Haikou development board. >>> >>> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> >> >> This works, thanks. >> >> Tested-by: Quentin Schulz <quentin.schulz@cherry.de> >> >> Now I have some questions I wasn't able to answer myself, maybe someone >> can provide some feedback on those :) >> >> We already have a gpio-keys for buttons on Haikou, c.f. >> https://elixir.bootlin.com/linux/v6.11/source/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#L22. >> Those signals are directly routed to the SoM and follow the Qseven standard. >> >> The same applies to PWRBTN# signal. >> >> However, here we have one gpio-keys for PWRBTN# in Puma DTSI and one >> gpio-keys for the buttons and sliders on Haikou devkit in Haikou DTS. >> >> I'm a bit undecided on where this should go. >> >> Having all button/slider signals following the Qseven standard in Puma >> DTSI and enable the gpio-keys only in the devkit would make sense to me, >> so that other baseboards could easily make use of it. However, things >> get complicated if the baseboard manufacturer decides to only implement >> **some** of the signals, for which we then need to remove some nodes >> from gpio-keys (and pinctrl entries) since gpio-keys doesn't check the >> "status" property in its child nodes (though that could be fixed). At >> which point, it's not entirely clear if having it in Puma DTSI is >> actually beneficial. >> >> Someone has an opinion/recommendation on that? > > I guess from a platform perspective nobody really cares, so as that is > "your" board, it comes down to a policy decision on your part ;-) . > > While pins follow the q7 standard, there may very well be some lax > handling of that standard in some places, and I guess gpio lines could > be re-used for something else if needed, as something like the lid-switch > is probably non-essential. > > Also a gpio-key input does not create that much code-overhead if > replicated, so personally I'd just stick the power-button with the other > buttons in the haikou dts. > > Which is also a way better thing than having multiple gpio-keys instances > that userspace then has to handle. > Yes, but this also means "code" duplication for whoever needs this for their baseboard, instead of just having to add a &gpio_keys { status = "okay"; }. I don't think there's a good solution here, so I would suggest we go with everything in Haikou's gpio-keys as Heiko suggested then, @Daniel if you agree can you send a v2 for that? Thanks! Quentin ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma 2024-09-30 9:11 ` Quentin Schulz @ 2024-09-30 9:21 ` Heiko Stübner 2024-09-30 12:21 ` Daniel Semkowicz 0 siblings, 1 reply; 6+ messages in thread From: Heiko Stübner @ 2024-09-30 9:21 UTC (permalink / raw) To: Daniel Semkowicz, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Quentin Schulz Cc: Dragan Simic, Farouk Bouabid, Quentin Schulz, Vahe Grigoryan, devicetree, linux-arm-kernel, linux-kernel, linux-rockchip Am Montag, 30. September 2024, 11:11:56 CEST schrieb Quentin Schulz: > Hi Heiko, > > On 9/30/24 10:49 AM, Heiko Stübner wrote: > > Hey Quentin, Daniel, > > > > Am Donnerstag, 26. September 2024, 14:34:30 CEST schrieb Quentin Schulz: > >> On 9/25/24 9:28 AM, Daniel Semkowicz wrote: > >>> There is a PWRBTN# input pin exposed on a Q7 connector. The pin > >>> is routed to a GPIO0_A1 through a diode. Q7 specification describes > >>> the PWRBTN# pin as a Power Button signal. > >>> Configure the pin as KEY_POWER, so it can function as power button and > >>> trigger device shutdown. > >>> Add the pin definition to RK3399 Puma dts, so it can be reused > >>> by derived platforms, but keep it disabled by default. > >>> > >>> Enable the power button input on Haikou development board. > >>> > >>> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > >> > >> This works, thanks. > >> > >> Tested-by: Quentin Schulz <quentin.schulz@cherry.de> > >> > >> Now I have some questions I wasn't able to answer myself, maybe someone > >> can provide some feedback on those :) > >> > >> We already have a gpio-keys for buttons on Haikou, c.f. > >> https://elixir.bootlin.com/linux/v6.11/source/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#L22. > >> Those signals are directly routed to the SoM and follow the Qseven standard. > >> > >> The same applies to PWRBTN# signal. > >> > >> However, here we have one gpio-keys for PWRBTN# in Puma DTSI and one > >> gpio-keys for the buttons and sliders on Haikou devkit in Haikou DTS. > >> > >> I'm a bit undecided on where this should go. > >> > >> Having all button/slider signals following the Qseven standard in Puma > >> DTSI and enable the gpio-keys only in the devkit would make sense to me, > >> so that other baseboards could easily make use of it. However, things > >> get complicated if the baseboard manufacturer decides to only implement > >> **some** of the signals, for which we then need to remove some nodes > >> from gpio-keys (and pinctrl entries) since gpio-keys doesn't check the > >> "status" property in its child nodes (though that could be fixed). At > >> which point, it's not entirely clear if having it in Puma DTSI is > >> actually beneficial. > >> > >> Someone has an opinion/recommendation on that? > > > > I guess from a platform perspective nobody really cares, so as that is > > "your" board, it comes down to a policy decision on your part ;-) . > > > > While pins follow the q7 standard, there may very well be some lax > > handling of that standard in some places, and I guess gpio lines could > > be re-used for something else if needed, as something like the lid-switch > > is probably non-essential. > > > > Also a gpio-key input does not create that much code-overhead if > > replicated, so personally I'd just stick the power-button with the other > > buttons in the haikou dts. > > > > Which is also a way better thing than having multiple gpio-keys instances > > that userspace then has to handle. > > > > Yes, but this also means "code" duplication for whoever needs this for > their baseboard, instead of just having to add a &gpio_keys { status = > "okay"; }. Yes :-) . gpio-keys is special in a way in that you could end up with a different set of enabled keys per baseboard - dependent on how closely it follows the standard. So if someone repurposed the lid-switch only, you'd start changing the core node again. Hence for the gpio-keys it's probably easier to define the set of keys in the baseboard. It's of course different for regulator-infrastructure and such. > I don't think there's a good solution here, so I would suggest we go > with everything in Haikou's gpio-keys as Heiko suggested then, @Daniel > if you agree can you send a v2 for that? I'll wait for v2 then. Heiko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma 2024-09-30 9:21 ` Heiko Stübner @ 2024-09-30 12:21 ` Daniel Semkowicz 0 siblings, 0 replies; 6+ messages in thread From: Daniel Semkowicz @ 2024-09-30 12:21 UTC (permalink / raw) To: Heiko Stübner Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Quentin Schulz, Dragan Simic, Farouk Bouabid, Quentin Schulz, Vahe Grigoryan, devicetree, linux-arm-kernel, linux-kernel, linux-rockchip On Mon, Sep 30, 2024 at 11:21 AM Heiko Stübner <heiko@sntech.de> wrote: > > Am Montag, 30. September 2024, 11:11:56 CEST schrieb Quentin Schulz: > > Hi Heiko, > > > > On 9/30/24 10:49 AM, Heiko Stübner wrote: > > > Hey Quentin, Daniel, > > > Hello Heiko, Hello Quentin, > > > Am Donnerstag, 26. September 2024, 14:34:30 CEST schrieb Quentin Schulz: > > >> On 9/25/24 9:28 AM, Daniel Semkowicz wrote: > > >>> There is a PWRBTN# input pin exposed on a Q7 connector. The pin > > >>> is routed to a GPIO0_A1 through a diode. Q7 specification describes > > >>> the PWRBTN# pin as a Power Button signal. > > >>> Configure the pin as KEY_POWER, so it can function as power button and > > >>> trigger device shutdown. > > >>> Add the pin definition to RK3399 Puma dts, so it can be reused > > >>> by derived platforms, but keep it disabled by default. > > >>> > > >>> Enable the power button input on Haikou development board. > > >>> > > >>> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com> > > >> > > >> This works, thanks. > > >> > > >> Tested-by: Quentin Schulz <quentin.schulz@cherry.de> > > >> > > >> Now I have some questions I wasn't able to answer myself, maybe someone > > >> can provide some feedback on those :) > > >> > > >> We already have a gpio-keys for buttons on Haikou, c.f. > > >> https://elixir.bootlin.com/linux/v6.11/source/arch/arm64/boot/dts/rockchip/rk3399-puma-haikou.dts#L22. > > >> Those signals are directly routed to the SoM and follow the Qseven standard. > > >> > > >> The same applies to PWRBTN# signal. > > >> > > >> However, here we have one gpio-keys for PWRBTN# in Puma DTSI and one > > >> gpio-keys for the buttons and sliders on Haikou devkit in Haikou DTS. > > >> > > >> I'm a bit undecided on where this should go. > > >> > > >> Having all button/slider signals following the Qseven standard in Puma > > >> DTSI and enable the gpio-keys only in the devkit would make sense to me, > > >> so that other baseboards could easily make use of it. However, things > > >> get complicated if the baseboard manufacturer decides to only implement > > >> **some** of the signals, for which we then need to remove some nodes > > >> from gpio-keys (and pinctrl entries) since gpio-keys doesn't check the > > >> "status" property in its child nodes (though that could be fixed). At > > >> which point, it's not entirely clear if having it in Puma DTSI is > > >> actually beneficial. > > >> > > >> Someone has an opinion/recommendation on that? > > > > > > I guess from a platform perspective nobody really cares, so as that is > > > "your" board, it comes down to a policy decision on your part ;-) . > > > > > > While pins follow the q7 standard, there may very well be some lax > > > handling of that standard in some places, and I guess gpio lines could > > > be re-used for something else if needed, as something like the lid-switch > > > is probably non-essential. > > > > > > Also a gpio-key input does not create that much code-overhead if > > > replicated, so personally I'd just stick the power-button with the other > > > buttons in the haikou dts. > > > > > > Which is also a way better thing than having multiple gpio-keys instances > > > that userspace then has to handle. > > > > > > > Yes, but this also means "code" duplication for whoever needs this for > > their baseboard, instead of just having to add a &gpio_keys { status = > > "okay"; }. > > Yes :-) . > > gpio-keys is special in a way in that you could end up with a different set > of enabled keys per baseboard - dependent on how closely it follows the > standard. > > So if someone repurposed the lid-switch only, you'd start changing the > core node again. Hence for the gpio-keys it's probably easier to define > the set of keys in the baseboard. > > It's of course different for regulator-infrastructure and such. > > > I don't think there's a good solution here, so I would suggest we go > > with everything in Haikou's gpio-keys as Heiko suggested then, @Daniel > > if you agree can you send a v2 for that? I'm fine with that. I will prepare v2 with such change. > > I'll wait for v2 then. > > Heiko > > Kind regards Daniel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-30 12:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-25 7:28 [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma Daniel Semkowicz 2024-09-26 12:34 ` Quentin Schulz 2024-09-30 8:49 ` Heiko Stübner 2024-09-30 9:11 ` Quentin Schulz 2024-09-30 9:21 ` Heiko Stübner 2024-09-30 12:21 ` Daniel Semkowicz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).