[PATCH 0/4] input: add CONFIG_INPUT_EVBUG debug option

[PATCH 0/4] input: add CONFIG_INPUT_EVBUG debug option

[Ahmad Fatoum]

Incorrectly specified or configured gpio-keys may cause the barebox
boot to abort. To help debugging such issues, add a CONFIG_INPUT_EVBUG
option similar to Linux, which will print a debug message every time
a key event is reported.

Ahmad Fatoum (4):
  input: record parent device at registration time
  input: add CONFIG_INPUT_EVBUG debug option
  input: imx-keypad: remove now-duplicate debug messages
  input: gpio-keys: drop now-superfluous struct gpio_keys::dev

 drivers/input/Kconfig        | 11 +++++++++++
 drivers/input/gpio_keys.c    |  6 ++----
 drivers/input/imx_keypad.c   |  7 +------
 drivers/input/input.c        | 11 +++++++++++
 drivers/input/usb_kbd.c      |  1 +
 drivers/input/virtio_input.c |  1 +
 include/input/input.h        |  1 +
 7 files changed, 28 insertions(+), 10 deletions(-)

-- 
2.39.2

[PATCH 1/4] input: record parent device at registration time

[Ahmad Fatoum]

The input core doesn't create a virtual device to represent the input
device. Still having a pointer to the parent device can be useful for
debugging purposes.

Add a parent member and start populating it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/input/gpio_keys.c    | 1 +
 drivers/input/imx_keypad.c   | 1 +
 drivers/input/usb_kbd.c      | 1 +
 drivers/input/virtio_input.c | 1 +
 include/input/input.h        | 1 +
 5 files changed, 5 insertions(+)

diff --git a/drivers/input/gpio_keys.c b/drivers/input/gpio_keys.c
index c23d20563c38..190865b7f244 100644
--- a/drivers/input/gpio_keys.c
+++ b/drivers/input/gpio_keys.c
@@ -162,6 +162,7 @@ static int __init gpio_keys_probe(struct device *dev)
 		gk->buttons[i].previous_state = gk->buttons[i].active_low;
 	}
 
+	gk->input.parent = dev;
 	ret = input_device_register(&gk->input);
 	if (ret)
 		return ret;
diff --git a/drivers/input/imx_keypad.c b/drivers/input/imx_keypad.c
index fc6b45b3cb62..2c003905a7d9 100644
--- a/drivers/input/imx_keypad.c
+++ b/drivers/input/imx_keypad.c
@@ -410,6 +410,7 @@ static int __init imx_keypad_probe(struct device *dev)
 	if (ret)
 		return ret;
 
+	keypad->input.parent = dev;
 	ret = input_device_register(&keypad->input);
 	if (ret)
 		return ret;
diff --git a/drivers/input/usb_kbd.c b/drivers/input/usb_kbd.c
index 86b48db2a72b..2e75aabf3da4 100644
--- a/drivers/input/usb_kbd.c
+++ b/drivers/input/usb_kbd.c
@@ -188,6 +188,7 @@ static int usb_kbd_probe(struct usb_device *usbdev,
 	} else
 		dev_dbg(&usbdev->dev, "poll keyboard via int ep\n");
 
+	data->input.parent = &usbdev->dev;
 	ret = input_device_register(&data->input);
 	if (ret) {
 		dev_err(&usbdev->dev, "can't register input\n");
diff --git a/drivers/input/virtio_input.c b/drivers/input/virtio_input.c
index 5c6849b561eb..655a9051726a 100644
--- a/drivers/input/virtio_input.c
+++ b/drivers/input/virtio_input.c
@@ -216,6 +216,7 @@ static int virtinput_probe(struct virtio_device *vdev)
 
 	virtio_device_ready(vdev);
 
+	vi->idev.parent = &vdev->dev;
 	err = input_device_register(&vi->idev);
 	if (err)
 		goto err_input_register;
diff --git a/include/input/input.h b/include/input/input.h
index d169c647bdaf..9445d20e566b 100644
--- a/include/input/input.h
+++ b/include/input/input.h
@@ -14,6 +14,7 @@ struct input_event {
 
 struct input_device {
 	struct list_head list;
+	struct device *parent;
 	DECLARE_BITMAP(keys, KEY_CNT);
 };
 
-- 
2.39.2

[PATCH 2/4] input: add CONFIG_INPUT_EVBUG debug option

[Ahmad Fatoum]

Misconfigured pull-ups may cause the barebox boot to abort. To help
debugging such issues, add a CONFIG_INPUT_EVBUG option similar to Linux,
which will print a debug message every time a key event is reported.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/input/Kconfig | 11 +++++++++++
 drivers/input/input.c | 11 +++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 52234c0ecc49..01e0b722c20f 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -12,6 +12,17 @@ config INPUT
 config INPUT_MATRIXKMAP
 	bool
 
+config INPUT_EVBUG
+        bool "Event debugging"
+        help
+          Say Y here if you have a problem with the input subsystem and
+          want all events (keypresses), to be output to
+          the barebox log. While this is useful for debugging, it's also
+          a security threat - your keypresses include your passwords, of
+          course and could be visible to userspace if pstore is configured.
+
+          If unsure, say N.
+
 config KEYBOARD_GPIO
 	bool "GPIO Buttons"
 	depends on GENERIC_GPIO
diff --git a/drivers/input/input.c b/drivers/input/input.c
index deef5ddd22a7..1a4792935114 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#define pr_fmt(fmt) "input: " fmt
+
 #include <common.h>
 #include <init.h>
 #include <kfifo.h>
@@ -33,6 +35,15 @@ void input_report_key_event(struct input_device *idev, unsigned int code, int va
 	if (code > KEY_MAX)
 		return;
 
+	/*
+	 * We don't use pr_debug here as we want to output the message
+	 * to the log, even if CONFIG_COMPILE_LOGLEVEL < MSG_DEBUG and
+	 * the DEBUG mcro wasn't defined for the file.
+	 */
+	if (IS_ENABLED(CONFIG_INPUT_EVBUG))
+		pr_print(MSG_DEBUG, "Event. Dev: %s, Type: %d, Code: %d, Value: %d\n",
+			 dev_name(idev->parent), EV_KEY, code, value);
+
 	if (value)
 		set_bit(code, idev->keys);
 	else
-- 
2.39.2

[PATCH 3/4] input: imx-keypad: remove now-duplicate debug messages

[Ahmad Fatoum]

With the recent addition of CONFIG_EVENT_EVBUG, the debug messages in
both i.MX keypad driver adds no extra value, so remove it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/input/imx_keypad.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/input/imx_keypad.c b/drivers/input/imx_keypad.c
index 2c003905a7d9..e57a884630aa 100644
--- a/drivers/input/imx_keypad.c
+++ b/drivers/input/imx_keypad.c
@@ -72,7 +72,6 @@
 struct imx_keypad {
 	struct input_device input;
 	struct clk *clk;
-	struct device *dev;
 	void __iomem *mmio_base;
 
 	struct poller_struct poller;
@@ -199,10 +198,6 @@ static void imx_keypad_fire_events(struct imx_keypad *keypad,
 
 			input_report_key_event(&keypad->input, keypad->keycodes[code],
 					       matrix_volatile_state[col] & (1 << row));
-
-			dev_dbg(keypad->dev, "Event code: %d, val: %d",
-				keypad->keycodes[code],
-				matrix_volatile_state[col] & (1 << row));
 		}
 	}
 }
@@ -367,7 +362,6 @@ static int __init imx_keypad_probe(struct device *dev)
 
 	keypad = xzalloc(sizeof(struct imx_keypad));
 
-	keypad->dev = dev;
 	iores = dev_request_mem_resource(dev, 0);
 	if (IS_ERR(iores))
 		return PTR_ERR(iores);
-- 
2.39.2

[PATCH 4/4] input: gpio-keys: drop now-superfluous struct gpio_keys::dev

[Ahmad Fatoum]

Now that gpio-keys populates the new struct input_device::parent member,
there is no need to duplicate the information in struct gpio_keys.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/input/gpio_keys.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/gpio_keys.c b/drivers/input/gpio_keys.c
index 190865b7f244..c897acf3bd02 100644
--- a/drivers/input/gpio_keys.c
+++ b/drivers/input/gpio_keys.c
@@ -31,7 +31,6 @@ struct gpio_keys {
 
 	struct poller_async poller;
 	struct input_device input;
-	struct device *dev;
 };
 
 static void gpio_key_poller(void *data)
@@ -60,7 +59,7 @@ static void gpio_key_poller(void *data)
 
 			gb->debounce_start = get_time_ns();
 			input_report_key_event(&gk->input, gb->code, pressed);
-			dev_dbg(gk->dev, "%s gpio(%d) as %d\n",
+			dev_dbg(gk->input.parent, "%s gpio(%d) as %d\n",
 				pressed ? "pressed" : "released", gb->gpio, gb->code);
 			gb->previous_state = val;
 		}
@@ -141,8 +140,6 @@ static int __init gpio_keys_probe(struct device *dev)
 
 	gk = xzalloc(sizeof(*gk));
 
-	gk->dev = dev;
-
 	if (dev->of_node)
 		ret = gpio_keys_probe_dt(gk, dev);
 	else
-- 
2.39.2

Re: [PATCH 3/4] input: imx-keypad: remove now-duplicate debug messages

[Ahmad Fatoum]

On 02.04.24 15:50, Ahmad Fatoum wrote:
> With the recent addition of CONFIG_EVENT_EVBUG, the debug messages in

s/CONFIG_INPUT_EVBUG/CONFIG_EVENT_EVBUG/

Please let me know if I should resend.

> both i.MX keypad driver adds no extra value, so remove it.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/input/imx_keypad.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/input/imx_keypad.c b/drivers/input/imx_keypad.c
> index 2c003905a7d9..e57a884630aa 100644
> --- a/drivers/input/imx_keypad.c
> +++ b/drivers/input/imx_keypad.c
> @@ -72,7 +72,6 @@
>  struct imx_keypad {
>  	struct input_device input;
>  	struct clk *clk;
> -	struct device *dev;
>  	void __iomem *mmio_base;
>  
>  	struct poller_struct poller;
> @@ -199,10 +198,6 @@ static void imx_keypad_fire_events(struct imx_keypad *keypad,
>  
>  			input_report_key_event(&keypad->input, keypad->keycodes[code],
>  					       matrix_volatile_state[col] & (1 << row));
> -
> -			dev_dbg(keypad->dev, "Event code: %d, val: %d",
> -				keypad->keycodes[code],
> -				matrix_volatile_state[col] & (1 << row));
>  		}
>  	}
>  }
> @@ -367,7 +362,6 @@ static int __init imx_keypad_probe(struct device *dev)
>  
>  	keypad = xzalloc(sizeof(struct imx_keypad));
>  
> -	keypad->dev = dev;
>  	iores = dev_request_mem_resource(dev, 0);
>  	if (IS_ERR(iores))
>  		return PTR_ERR(iores);

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/4] input: add CONFIG_INPUT_EVBUG debug option

[Sascha Hauer]

On Tue, 02 Apr 2024 15:50:31 +0200, Ahmad Fatoum wrote:
> Incorrectly specified or configured gpio-keys may cause the barebox
> boot to abort. To help debugging such issues, add a CONFIG_INPUT_EVBUG
> option similar to Linux, which will print a debug message every time
> a key event is reported.
> 
> Ahmad Fatoum (4):
>   input: record parent device at registration time
>   input: add CONFIG_INPUT_EVBUG debug option
>   input: imx-keypad: remove now-duplicate debug messages
>   input: gpio-keys: drop now-superfluous struct gpio_keys::dev
> 
> [...]

Applied, thanks!

[1/4] input: record parent device at registration time
      https://git.pengutronix.de/cgit/barebox/commit/?id=ec521e914b41 (link may not be stable)
[2/4] input: add CONFIG_INPUT_EVBUG debug option
      https://git.pengutronix.de/cgit/barebox/commit/?id=40db06326f87 (link may not be stable)
[3/4] input: imx-keypad: remove now-duplicate debug messages
      https://git.pengutronix.de/cgit/barebox/commit/?id=648bfcfb9684 (link may not be stable)
[4/4] input: gpio-keys: drop now-superfluous struct gpio_keys::dev
      https://git.pengutronix.de/cgit/barebox/commit/?id=47b2ae8489ed (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH 3/4] input: imx-keypad: remove now-duplicate debug messages

[Sascha Hauer]

On Tue, Apr 02, 2024 at 04:06:49PM +0200, Ahmad Fatoum wrote:
> On 02.04.24 15:50, Ahmad Fatoum wrote:
> > With the recent addition of CONFIG_EVENT_EVBUG, the debug messages in
> 
> s/CONFIG_INPUT_EVBUG/CONFIG_EVENT_EVBUG/
> 
> Please let me know if I should resend.

Fixed while applying.

> 
> > both i.MX keypad driver adds no extra value, so remove it.

Also removed the 'both'

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |