[PATCHv2] input: ipaq-micro-keys: simplify allocation

[PATCHv2] input: ipaq-micro-keys: simplify allocation

[Rosen Penev]

Embed the keycode array in the struct to have a single allocation.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 v2: use regular array instead of flexible array member.
 drivers/input/keyboard/ipaq-micro-keys.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
index ca7ec054b1ce..695ef3c2081a 100644
--- a/drivers/input/keyboard/ipaq-micro-keys.c
+++ b/drivers/input/keyboard/ipaq-micro-keys.c
@@ -20,12 +20,6 @@
 #include <linux/platform_device.h>
 #include <linux/mfd/ipaq-micro.h>

-struct ipaq_micro_keys {
-	struct ipaq_micro *micro;
-	struct input_dev *input;
-	u16 *codes;
-};
-
 static const u16 micro_keycodes[] = {
 	KEY_RECORD,		/* 1:  Record button			*/
 	KEY_CALENDAR,		/* 2:  Calendar				*/
@@ -38,6 +32,12 @@ static const u16 micro_keycodes[] = {
 	KEY_DOWN,		/* 9:  Down				*/
 };

+struct ipaq_micro_keys {
+	struct ipaq_micro *micro;
+	struct input_dev *input;
+	u16 codes[ARRAY_SIZE(micro_keycodes)];
+};
+
 static void micro_key_receive(void *data, int len, unsigned char *msg)
 {
 	struct ipaq_micro_keys *keys = data;
@@ -102,10 +102,7 @@ static int micro_key_probe(struct platform_device *pdev)

 	keys->input->keycodesize = sizeof(micro_keycodes[0]);
 	keys->input->keycodemax = ARRAY_SIZE(micro_keycodes);
-	keys->codes = devm_kmemdup_array(&pdev->dev, micro_keycodes, keys->input->keycodemax,
-					 keys->input->keycodesize, GFP_KERNEL);
-	if (!keys->codes)
-		return -ENOMEM;
+	memcpy(keys->codes, micro_keycodes, sizeof(keys->codes));

 	keys->input->keycode = keys->codes;

--
2.54.0

Re: [PATCHv2] input: ipaq-micro-keys: simplify allocation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential deadlock in `micro_key_start()` and `micro_key_stop()` due to taking a spinlock without disabling interrupts, which is also taken in a hardirq handler.
--

commit 59a420f5a26f394b8f84d4e056f388e517d6f808
Author: Rosen Penev <rosenp@gmail.com>

input: ipaq-micro-keys: simplify allocation

This commit simplifies the keycode array allocation in the ipaq-micro-keys
driver. By embedding the array directly into the device structure, it removes
the need for a separate devm_kmemdup_array() call during probe.

> diff --git a/drivers/input/keyboard/ipaq-micro-keys.c b/drivers/input/keyboard/ipaq-micro-keys.c
> index ca7ec054b1cef..695ef3c2081a4 100644
> --- a/drivers/input/keyboard/ipaq-micro-keys.c
> +++ b/drivers/input/keyboard/ipaq-micro-keys.c

[ ... ]

> @@ -102,10 +102,7 @@ static int micro_key_probe(struct platform_device *pdev)
>  
>  	keys->input->keycodesize = sizeof(micro_keycodes[0]);
>  	keys->input->keycodemax = ARRAY_SIZE(micro_keycodes);
> -	keys->codes = devm_kmemdup_array(&pdev->dev, micro_keycodes, keys->input->keycodemax,
> -					 keys->input->keycodesize, GFP_KERNEL);
> -	if (!keys->codes)
> -		return -ENOMEM;
> +	memcpy(keys->codes, micro_keycodes, sizeof(keys->codes));
>  
>  	keys->input->keycode = keys->codes;

[Severity: High]
This isn't a bug introduced by this patch, but could the lock usage in
micro_key_start() and micro_key_stop() cause a deadlock if an interrupt
fires?

Looking at drivers/input/keyboard/ipaq-micro-keys.c, the micro->lock is
acquired in process context without disabling interrupts:

micro_key_start() {
    guard(spinlock)(&keys->micro->lock);
    ...
}

If a serial interrupt fires on the same CPU while the process context holds
the lock, the interrupt handler micro_serial_isr() runs, processes the
character, and calls micro_rx_msg().

micro_rx_msg() in drivers/mfd/ipaq-micro.c then attempts to acquire the same
lock in hardirq context:

    spin_lock(&micro->lock);

Does this sequence lead to a lock inversion deadlock? Should the process
context functions use guard(spinlock_irqsave) or guard(spinlock_irq)
instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609213532.25181-1-rosenp@gmail.com?part=1