[PATCH 0/2] input: Supersede input_key_get_status() with input_is_key_pressed()

[PATCH 0/2] input: Supersede input_key_get_status() with input_is_key_pressed()

[Jonas Rebmann]

Reading a GPIO button via gpio-keys can be preferable over direct GPIO
access as gpio-keys abstracts away from GPIO hardware via the
devicetree, consistently across barebox and linux.

Trying to read a single key state analogous to prt_imx6_init_prtvt7
failed as input_key_get_status should have been provided with the
keycode in question plus one to ensure a large enough bitfield to
contain the desired key state.

The correct way of reading a single key state via the
input_key_get_status() interface is as complicated as

    bool is_button_pressed(int key) {
        unsigned long *keys;
        bool pressed;

        keys = bitmap_xzalloc(key + 1);
        input_key_get_status(keys, key + 1);

        pressed = test_bit(key, keys);

        free(keys);
        return pressed;
   }

This series implements a simple way to read the state of a single key
directly in the input driver.

As this can easily be invoked twice to check for a key combination,
replace this single occurence of input_key_get_status() with
input_is_key_pressed().

While those changes where thoroughly tested on a board with boardcode
very similar to protonic-imx6, the change to protonic-imx6 itself is
untested.

I suppose we have to keep input_key_get_status() for now for backwards
compatibility with downstream board code?

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Jonas Rebmann (2):
      input: add input_is_key_pressed() to read single key
      ARM: boards: protonic-imx6: use input_is_key_pressed()

 arch/arm/boards/protonic-imx6/board.c |  8 +-------
 drivers/input/input.c                 | 14 ++++++++++++++
 include/input/input.h                 |  1 +
 3 files changed, 16 insertions(+), 7 deletions(-)
---
base-commit: 7a178f01f6e25474a5eb6e071ca479076b8d4d92
change-id: 20260507-input_is_key_pressed-4472a8a29980

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

[PATCH 1/2] input: add input_is_key_pressed() to read single key

[Jonas Rebmann]

input_key_get_status() while being powerful and able to read all key
states at once, is difficult to use correctly, as the single incorrect
usage in upstream code shows.

Reading multiple key states at once, for any meaningful number of keys
combined (typically up to three) provides no advantage over checking the
keys sequentially.

Provide a simple input_is_key_pressed() function that checks a single
keycode.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/input/input.c | 14 ++++++++++++++
 include/input/input.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index e5509bf90d..ea3f4ef255 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -84,6 +84,20 @@ void input_key_get_status(unsigned long *keys, int bits)
 		bitmap_or(keys, keys, idev->keys, bits);
 }
 
+bool input_is_key_pressed(int key)
+{
+	struct input_device *idev;
+	bool pressed = false;
+
+	if (key > KEY_MAX)
+		return false;
+
+	list_for_each_entry(idev, &input_devices, list)
+		pressed = pressed || test_bit(key, idev->keys);
+
+	return pressed;
+}
+
 struct input_console {
 	struct console_device console;
 	struct input_notifier notifier;
diff --git a/include/input/input.h b/include/input/input.h
index 9445d20e56..3e03eb9a65 100644
--- a/include/input/input.h
+++ b/include/input/input.h
@@ -24,6 +24,7 @@ int input_device_register(struct input_device *);
 void input_device_unregister(struct input_device *);
 
 void input_key_get_status(unsigned long *keys, int bits);
+bool input_is_key_pressed(int key);
 
 struct input_notifier {
 	void (*notify)(struct input_notifier *in, struct input_event *event);

-- 
2.53.0.610.g30c4861dc6

[PATCH 2/2] ARM: boards: protonic-imx6: use input_is_key_pressed()

[Jonas Rebmann]

input_key_get_status() prepares a bitmap of the provided size. This
means that a keycode provided here is not actually contained in the
resulting bitmap and the test is guaranteed to fail at least if the
provided keycode is a multiple of sizeof(long)*8.

input_key_get_status(keys, KEY_CYCLEWINDOWS + 1) would have been
correct.

The use of xzalloc to allocate the bitmap is also incorrect,
bitmap_xzalloc(KEY_CYCLEWINDOWS + 1) would have been correct.

As bitmaps are composed of longs, the underlying array is always sized a
multiple of sizeof(long). Although the allocations are unaffected by
this as they are a multiple of sizeof(long) too, xzalloc only zeroes the
specified number of bytes resulting in a potentially nonzero bitmap.

Avoid those troubles altogether and use input_is_key_pressed() instead.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 arch/arm/boards/protonic-imx6/board.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/arm/boards/protonic-imx6/board.c b/arch/arm/boards/protonic-imx6/board.c
index 3bb2632693..174ccf70bd 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -740,21 +740,15 @@ static int prt_imx6_init_kvg_yaco(struct prt_imx6_priv *priv)
 
 static int prt_imx6_init_prtvt7(struct prt_imx6_priv *priv)
 {
-	unsigned long *keys;
-
 	of_devices_ensure_probed_by_compatible("gpio-keys");
 
 	/*
 	 * Prefer USB-boot and enable autoboot with timeout when CYCLE-F6 key
 	 * combination is pressed.
 	 */
-	keys = xzalloc((KEY_CYCLEWINDOWS / 8) + 1);
-	input_key_get_status(keys, KEY_CYCLEWINDOWS);
-
-	if (!(test_bit(KEY_CYCLEWINDOWS, keys) && test_bit(KEY_F6, keys)))
+	if (!(input_is_key_pressed(KEY_CYCLEWINDOWS) && input_is_key_pressed(KEY_F6)))
 		priv->no_usb_check = 1;
 
-	free(keys);
 	return 0;
 }
 

-- 
2.53.0.610.g30c4861dc6