Re: [PATCH] arm64: dts: rockchip: Add power button for RK3399 Puma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>

This works, thanks.

Tested-by: Quentin Schulz <quentin.schulz@xxxxxxxxx>

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux