Re: [PATCH] dm: button: support remapping phone keys

[Dragan Simic]

Hello Caleb,

On 2024-07-14 21:49, Caleb Connolly wrote:

We don't have audio support in U-Boot, but we do have boot menus. Add an

option to re-map the volume and power buttons to up/down/enter so that
in situations where these are the only available buttons (such as on
mobile phones) it's still possible to navigate menus built in U-Boot or
an external EFI app like GRUB or systemd-boot.

Signed-off-by: Caleb Connolly <caleb.conno...@linaro.org>
---
Cc: u-boot-q...@groups.io

Very nice, thanks for this patch!  Looking good to me, with a few
suggestions available below.  Anyway, please feel free to add:

Reviewed-by: Dragan Simic <dsi...@manjaro.org>

---
 drivers/button/Kconfig         | 11 +++++++++++
 drivers/button/button-uclass.c | 22 +++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 3918b05ae03e..6cae16fcc8bf 100644
--- a/drivers/button/Kconfig
+++ b/drivers/button/Kconfig
@@ -8,8 +8,19 @@ config BUTTON

U-Boot provides a uclass API to implement this feature. Button drivers can provide access to board-specific buttons. Use of the device tree

          for configuration is encouraged.

+config BUTTON_REMAP_PHONE_KEYS
+       bool "Remap phone keys for navigation"
+       depends on BUTTON
+       help

+ Enable remapping of phone keys to navigation keys. This is useful for

+         devices with phone keys that are not used in U-Boot. The phone keys
+         are remapped to the following navigation keys:
+         - Volume up: Up
+         - Volume down: Down
+         - Power: Enter
+

Frankly, "phone keys" sounds a bit strange to me, maybe because there
are also tablets that have the same style of reduced-set keys.  Thus,
I'd suggest that the following language is used:

- "BUTTON_REMAP_PHONE_KEYS" instead of "BUTTON_REMAP_REDUCED_KEYS"
- "reduced smartphone-style keys" instead of "phone keys"

Using "reduced" would also allow us to have this remapping logic more
easily extended to also cover some other buttons found on some other
devices with reduced-set keys.

 config BUTTON_ADC
        bool "Button adc"
        depends on BUTTON
        depends on ADC

diff --git a/drivers/button/button-uclass.c b/drivers/button/button-uclass.c

index cda243389df3..729983d58701 100644
--- a/drivers/button/button-uclass.c
+++ b/drivers/button/button-uclass.c
@@ -9,8 +9,9 @@

 #include <button.h>
 #include <dm.h>
 #include <dm/uclass-internal.h>
+#include <dt-bindings/input/linux-event-codes.h>

 int button_get_by_label(const char *label, struct udevice **devp)
 {
        struct udevice *dev;

@@ -36,16 +37,35 @@ enum button_state_t button_get_state(struct udevice *dev)

        return ops->get_state(dev);
 }

+static int button_remap_phone_keys(int code)

Pretty much the same suggestion as above applies here.

+{
+       switch (code) {
+       case KEY_VOLUMEUP:
+               return KEY_UP;
+       case KEY_VOLUMEDOWN:
+               return KEY_DOWN;
+       case KEY_POWER:
+               return KEY_ENTER;
+       default:
+               return code;
+       }
+}
+
 int button_get_code(struct udevice *dev)
 {
        struct button_ops *ops = button_get_ops(dev);
+       int code;

        if (!ops->get_code)
                return -ENOSYS;

-       return ops->get_code(dev);
+       code = ops->get_code(dev);
+       if (CONFIG_IS_ENABLED(BUTTON_REMAP_PHONE_KEYS))
+               return button_remap_phone_keys(code);
+       else
+               return code;
 }

 UCLASS_DRIVER(button) = {
        .id             = UCLASS_BUTTON,