[PATCHv4 0/4] Lost key-up interrupt handling for omap4-keypad

Hi,

This series updates omap4-keypad to disable unused long interrupts, and
implements the missing parts for the lost key-up interrupt quirk as
described in the silicon errata pdf. It also simplifies the probe to
use devm.

Regards,

Tony


Changes since v3:

- Use PM runtime to check the last pressed key is not stuck

- Simplify probe with devm

Changes since v2:

- Drop bogus level change, that already comes from device tree

- Scan keyboard in two phases and simplify using a bitmask

- Use mod_delayed_work and cancel_delayed_work_sync for the quirk

Tony Lindgren (4):
  Input: omap4-keypad - disable unused long interrupts
  Input: omap4-keypad - scan keys in two phases and simplify with
    bitmask
  Input: omap4-keypad - use PM runtime to check keys for errata
  Input: omap4-keypad - simplify probe with devm

 drivers/input/keyboard/omap4-keypad.c | 255 +++++++++++++++++---------
 1 file changed, 170 insertions(+), 85 deletions(-)

-- 
2.30.0

We are not using the long events and they produce extra interrupts.
Let's not enable them at all.

Note that also the v3.0.8 Linux Android kernel has long interrupts
disabled.

Cc: Arthur Demchenkov <redacted>
Cc: Carl Philipp Klemm <redacted>
Cc: Merlijn Wajer <redacted>
Cc: Pavel Machek <redacted>
Cc: ruleh <redacted>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/keyboard/omap4-keypad.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c

@@ -176,10 +176,9 @@ static int omap4_keypad_open(struct input_dev *input)
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
 			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
-			OMAP4_DEF_IRQENABLE_EVENTEN |
-				OMAP4_DEF_IRQENABLE_LONGKEY);
+			OMAP4_DEF_IRQENABLE_EVENTEN);
 	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE,
-			OMAP4_DEF_WUP_EVENT_ENA | OMAP4_DEF_WUP_LONG_KEY_ENA);
+			OMAP4_DEF_WUP_EVENT_ENA);
 
 	enable_irq(keypad_data->irq);
 

-- 
2.30.0

Because of errata i689 the keyboard can idle with state where no key
up interrupts are seen until after the next key press.

This means we need to first check for any lost key up events before
scanning for new down events.

For example, rapidly pressing shift-shift-j can sometimes produce a J
instead of j. Let's fix the issue by scanning the keyboard in two
phases. First we scan for any key up events that we may have missed,
and then we scan for key down events.

Let's also simplify things with for_each_set_bit() as suggested by
Dmitry Torokhov [off-list ref].

Cc: Arthur Demchenkov <redacted>
Cc: Carl Philipp Klemm <redacted>
Cc: Merlijn Wajer <redacted>
Cc: Pavel Machek <redacted>
Cc: ruleh <redacted>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/keyboard/omap4-keypad.c | 70 ++++++++++++++++++---------
 1 file changed, 47 insertions(+), 23 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c

@@ -78,7 +78,7 @@ struct omap4_keypad {
 	u32 irqreg_offset;
 	unsigned int row_shift;
 	bool no_autorepeat;
-	unsigned char key_state[8];
+	u64 keys;
 	unsigned short *keymap;
 };
 

@@ -107,6 +107,41 @@ static void kbd_write_irqreg(struct omap4_keypad *keypad_data,
 		     keypad_data->base + keypad_data->irqreg_offset + offset);
 }
 
+static int omap4_keypad_scan_state(struct omap4_keypad *keypad_data, u64 keys,
+				   bool down)
+{
+	struct input_dev *input_dev = keypad_data->input;
+	unsigned int col, row, code;
+	DECLARE_BITMAP(mask, 64);
+	unsigned long bit;
+	int events = 0;
+	bool key_down;
+	u64 changed;
+
+	changed = keys ^ keypad_data->keys;
+	bitmap_from_u64(mask, changed);
+
+	for_each_set_bit(bit, mask, keypad_data->rows * BITS_PER_BYTE) {
+		row = bit / BITS_PER_BYTE;
+		col = bit % BITS_PER_BYTE;
+		code = MATRIX_SCAN_CODE(row, col, keypad_data->row_shift);
+
+		if (BIT_ULL(bit) & keys)
+			key_down = true;
+		else
+			key_down = false;
+
+		if (key_down != down)
+			continue;
+
+		input_event(input_dev, EV_MSC, MSC_SCAN, code);
+		input_report_key(input_dev, keypad_data->keymap[code],
+				 key_down);
+		events++;
+	}
+
+	return events;
+}
 
 /* Interrupt handlers */
 static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)

@@ -123,34 +158,23 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
 {
 	struct omap4_keypad *keypad_data = dev_id;
 	struct input_dev *input_dev = keypad_data->input;
-	unsigned char key_state[ARRAY_SIZE(keypad_data->key_state)];
-	unsigned int col, row, code, changed;
-	u32 *new_state = (u32 *) key_state;
+	int keys_up, keys_down;
+	u32 low, high;
+	u64 keys;
 
-	*new_state = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
-	*(new_state + 1) = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
+	low = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
+	high = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
+	keys = low | (u64)high << 32;
 
-	for (row = 0; row < keypad_data->rows; row++) {
-		changed = key_state[row] ^ keypad_data->key_state[row];
-		if (!changed)
-			continue;
+	/* Scan for key up events for lost key-up interrupts */
+	keys_up = omap4_keypad_scan_state(keypad_data, keys, false);
 
-		for (col = 0; col < keypad_data->cols; col++) {
-			if (changed & (1 << col)) {
-				code = MATRIX_SCAN_CODE(row, col,
-						keypad_data->row_shift);
-				input_event(input_dev, EV_MSC, MSC_SCAN, code);
-				input_report_key(input_dev,
-						 keypad_data->keymap[code],
-						 key_state[row] & (1 << col));
-			}
-		}
-	}
+	/* Scan for key down events */
+	keys_down = omap4_keypad_scan_state(keypad_data, keys, true);
 
 	input_sync(input_dev);
 
-	memcpy(keypad_data->key_state, key_state,
-		sizeof(keypad_data->key_state));
+	keypad_data->keys = keys;
 
 	/* clear pending interrupts */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,

-- 
2.30.0

Simplify probe with devm.

Cc: Arthur Demchenkov <redacted>
Cc: Carl Philipp Klemm <redacted>
Cc: Merlijn Wajer <redacted>
Cc: Pavel Machek <redacted>
Cc: ruleh <redacted>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/keyboard/omap4-keypad.c | 81 ++++++++++-----------------
 1 file changed, 30 insertions(+), 51 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c

@@ -341,7 +341,8 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	keypad_data = kzalloc(sizeof(struct omap4_keypad), GFP_KERNEL);
+	keypad_data = devm_kzalloc(&pdev->dev, sizeof(struct omap4_keypad),
+				   GFP_KERNEL);
 	if (!keypad_data) {
 		dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
 		return -ENOMEM;

@@ -352,20 +353,20 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 
 	error = omap4_keypad_parse_dt(&pdev->dev, keypad_data);
 	if (error)
-		goto err_free_keypad;
+		return error;
 
-	res = request_mem_region(res->start, resource_size(res), pdev->name);
+	res = devm_request_mem_region(&pdev->dev, res->start,
+				      resource_size(res), pdev->name);
 	if (!res) {
 		dev_err(&pdev->dev, "can't request mem region\n");
-		error = -EBUSY;
-		goto err_free_keypad;
+		return -EBUSY;
 	}
 
-	keypad_data->base = ioremap(res->start, resource_size(res));
+	keypad_data->base = devm_ioremap(&pdev->dev, res->start,
+					 resource_size(res));
 	if (!keypad_data->base) {
 		dev_err(&pdev->dev, "can't ioremap mem resource\n");
-		error = -ENOMEM;
-		goto err_release_mem;
+		return -ENOMEM;
 	}
 
 	pm_runtime_use_autosuspend(&pdev->dev);

@@ -379,20 +380,19 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	error = pm_runtime_get_sync(&pdev->dev);
 	if (error) {
 		dev_err(&pdev->dev, "pm_runtime_get_sync() failed\n");
-		pm_runtime_put_noidle(&pdev->dev);
-	} else {
-		error = omap4_keypad_check_revision(&pdev->dev,
-						    keypad_data);
-		if (!error) {
-			/* Ensure device does not raise interrupts */
-			omap4_keypad_stop(keypad_data);
-		}
+		return error;
 	}
+
+	error = omap4_keypad_check_revision(&pdev->dev,
+					    keypad_data);
 	if (error)
 		goto err_pm_disable;
 
+	/* Ensure device does not raise interrupts */
+	omap4_keypad_stop(keypad_data);
+
 	/* input device allocation */
-	keypad_data->input = input_dev = input_allocate_device();
+	keypad_data->input = input_dev = devm_input_allocate_device(&pdev->dev);
 	if (!input_dev) {
 		error = -ENOMEM;
 		goto err_pm_disable;

@@ -416,13 +416,13 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 
 	keypad_data->row_shift = get_count_order(keypad_data->cols);
 	max_keys = keypad_data->rows << keypad_data->row_shift;
-	keypad_data->keymap = kcalloc(max_keys,
-				      sizeof(keypad_data->keymap[0]),
-				      GFP_KERNEL);
+	keypad_data->keymap = devm_kcalloc(&pdev->dev, max_keys,
+					   sizeof(keypad_data->keymap[0]),
+					   GFP_KERNEL);
 	if (!keypad_data->keymap) {
 		dev_err(&pdev->dev, "Not enough memory for keymap\n");
 		error = -ENOMEM;
-		goto err_free_input;
+		goto err_pm_disable;
 	}
 
 	error = matrix_keypad_build_keymap(NULL, NULL,

@@ -430,21 +430,23 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 					   keypad_data->keymap, input_dev);
 	if (error) {
 		dev_err(&pdev->dev, "failed to build keymap\n");
-		goto err_free_keymap;
+		goto err_pm_disable;
 	}
 
-	error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
-				     omap4_keypad_irq_thread_fn, IRQF_ONESHOT,
-				     "omap4-keypad", keypad_data);
+	error = devm_request_threaded_irq(&pdev->dev, keypad_data->irq,
+					  omap4_keypad_irq_handler,
+					  omap4_keypad_irq_thread_fn,
+					  IRQF_ONESHOT, "omap4-keypad",
+					  keypad_data);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register interrupt\n");
-		goto err_free_keymap;
+		goto err_pm_disable;
 	}
 
 	error = input_register_device(keypad_data->input);
 	if (error < 0) {
 		dev_err(&pdev->dev, "failed to register input device\n");
-		goto err_free_irq;
+		goto err_pm_disable;
 	}
 
 	device_init_wakeup(&pdev->dev, true);

@@ -460,46 +462,23 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 
 	return 0;
 
-err_free_irq:
-	free_irq(keypad_data->irq, keypad_data);
-err_free_keymap:
-	kfree(keypad_data->keymap);
-err_free_input:
-	input_free_device(input_dev);
 err_pm_disable:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-	iounmap(keypad_data->base);
-err_release_mem:
-	release_mem_region(res->start, resource_size(res));
-err_free_keypad:
-	kfree(keypad_data);
+
 	return error;
 }
 
 static int omap4_keypad_remove(struct platform_device *pdev)
 {
 	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
-	struct resource *res;
 
 	dev_pm_clear_wake_irq(&pdev->dev);
-
-	free_irq(keypad_data->irq, keypad_data);
-
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-
 	input_unregister_device(keypad_data->input);
 
-	iounmap(keypad_data->base);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-
-	kfree(keypad_data->keymap);
-	kfree(keypad_data);
-
 	return 0;
 }
 

-- 
2.30.0

We are still missing handling for errata i689 related issues for the
case where we never see a key up interrupt for the last pressed key.

To fix the issue, we must scan the key state again after the keyboard
controller has idled to check if a key up event was missed. This is
described in the omap4 silicon errata documentation for Errata ID i689
"1.32 Keyboard Key Up Event Can Be Missed":

"When a key is released for a time shorter than the debounce time,
 in-between 2 key press (KP1 and KP2), the keyboard state machine will go
 to idle mode and will never detect the key release (after KP1, and also
 after KP2), and thus will never generate a new IRQ indicating the key
 release."

We can use PM runtime autosuspend features to check the keyboard state
again after it enters idle. We have the hardware support for clock
auto gating, and the keyboard is capable of generating wake-up events
in runtime suspended state.

Cc: Arthur Demchenkov <redacted>
Cc: Carl Philipp Klemm <redacted>
Cc: Merlijn Wajer <redacted>
Cc: Pavel Machek <redacted>
Cc: ruleh <redacted>
Cc: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/keyboard/omap4-keypad.c | 105 +++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 11 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c

@@ -60,6 +60,7 @@
 	((((dbms) * 1000) / ((1 << ((ptv) + 1)) * (1000000 / 32768))) - 1)
 #define OMAP4_VAL_DEBOUNCINGTIME_16MS					\
 	OMAP4_KEYPAD_DEBOUNCINGTIME_MS(16, OMAP4_KEYPAD_PTV_DIV_128)
+#define OMAP4_KEYPAD_AUTOIDLE_MS	50	/* Approximate measured time */
 
 enum {
 	KBD_REVISION_OMAP4 = 0,

@@ -71,6 +72,7 @@ struct omap4_keypad {
 
 	void __iomem *base;
 	unsigned int irq;
+	struct mutex lock;		/* for key scan */
 
 	unsigned int rows;
 	unsigned int cols;

@@ -154,17 +156,19 @@ static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
+static bool omap4_keypad_scan_keys(struct omap4_keypad *keypad_data, bool clear)
 {
-	struct omap4_keypad *keypad_data = dev_id;
 	struct input_dev *input_dev = keypad_data->input;
 	int keys_up, keys_down;
 	u32 low, high;
-	u64 keys;
+	u64 keys = 0;
 
-	low = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
-	high = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
-	keys = low | (u64)high << 32;
+	mutex_lock(&keypad_data->lock);
+	if (!clear) {
+		low = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
+		high = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
+		keys = low | (u64)high << 32;
+	}
 
 	/* Scan for key up events for lost key-up interrupts */
 	keys_up = omap4_keypad_scan_state(keypad_data, keys, false);

@@ -176,18 +180,49 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
 
 	keypad_data->keys = keys;
 
+	mutex_unlock(&keypad_data->lock);
+
+	return keys_down;
+}
+
+static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
+{
+	struct omap4_keypad *keypad_data = dev_id;
+	struct device *dev = keypad_data->input->dev.parent;
+	bool down_events;
+	int error;
+
+	error = pm_runtime_get_sync(dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(dev);
+
+		return IRQ_NONE;
+	}
+
+	down_events = omap4_keypad_scan_keys(keypad_data, false);
+
 	/* clear pending interrupts */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
 			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
 
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return IRQ_HANDLED;
 }
 
 static int omap4_keypad_open(struct input_dev *input)
 {
 	struct omap4_keypad *keypad_data = input_get_drvdata(input);
+	struct device *dev = input->dev.parent;
+	int error;
 
-	pm_runtime_get_sync(input->dev.parent);
+	error = pm_runtime_get_sync(dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(dev);
+
+		return error;
+	}
 
 	disable_irq(keypad_data->irq);
 

@@ -206,6 +241,9 @@ static int omap4_keypad_open(struct input_dev *input)
 
 	enable_irq(keypad_data->irq);
 
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
 	return 0;
 }
 

@@ -223,14 +261,23 @@ static void omap4_keypad_stop(struct omap4_keypad *keypad_data)
 
 static void omap4_keypad_close(struct input_dev *input)
 {
-	struct omap4_keypad *keypad_data;
+	struct omap4_keypad *keypad_data = input_get_drvdata(input);
+	struct device *dev = input->dev.parent;
+	int error;
+
+	error = pm_runtime_get_sync(dev);
+	if (error < 0) {
+		pm_runtime_put_noidle(dev);
+
+		return;
+	}
 
-	keypad_data = input_get_drvdata(input);
 	disable_irq(keypad_data->irq);
 	omap4_keypad_stop(keypad_data);
 	enable_irq(keypad_data->irq);
 
-	pm_runtime_put_sync(input->dev.parent);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 static int omap4_keypad_parse_dt(struct device *dev,

@@ -301,6 +348,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	}
 
 	keypad_data->irq = irq;
+	mutex_init(&keypad_data->lock);
 
 	error = omap4_keypad_parse_dt(&pdev->dev, keypad_data);
 	if (error)

@@ -320,6 +368,8 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 		goto err_release_mem;
 	}
 
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, OMAP4_KEYPAD_AUTOIDLE_MS);
 	pm_runtime_enable(&pdev->dev);
 
 	/*

@@ -337,7 +387,6 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 			/* Ensure device does not raise interrupts */
 			omap4_keypad_stop(keypad_data);
 		}
-		pm_runtime_put_sync(&pdev->dev);
 	}
 	if (error)
 		goto err_pm_disable;

@@ -406,6 +455,9 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, keypad_data);
 
+	pm_runtime_mark_last_busy(&pdev->dev);
+	pm_runtime_put_autosuspend(&pdev->dev);
+
 	return 0;
 
 err_free_irq:

@@ -415,6 +467,8 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 err_free_input:
 	input_free_device(input_dev);
 err_pm_disable:
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	iounmap(keypad_data->base);
 err_release_mem:

@@ -433,6 +487,7 @@ static int omap4_keypad_remove(struct platform_device *pdev)
 
 	free_irq(keypad_data->irq, keypad_data);
 
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	input_unregister_device(keypad_data->input);

@@ -454,6 +509,34 @@ static const struct of_device_id omap_keypad_dt_match[] = {
 };
 MODULE_DEVICE_TABLE(of, omap_keypad_dt_match);
 
+/*
+ * Errata ID i689 "1.32 Keyboard Key Up Event Can Be Missed".
+ * Interrupt may not happen for key-up events.
+ */
+static int __maybe_unused omap4_keypad_runtime_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
+	bool events;
+	u32 active;
+
+	active = kbd_readl(keypad_data, OMAP4_KBD_STATEMACHINE);
+	if (active) {
+		pm_runtime_mark_last_busy(dev);
+		return -EBUSY;
+	}
+
+	events = omap4_keypad_scan_keys(keypad_data, true);
+	if (events)
+		dev_info(dev, "cleared stuck events on idle\n");
+
+	return 0;
+}
+
+static const struct dev_pm_ops omap4_keypad_pm_ops = {
+	SET_RUNTIME_PM_OPS(omap4_keypad_runtime_suspend, NULL, NULL)
+};
+
 static struct platform_driver omap4_keypad_driver = {
 	.probe		= omap4_keypad_probe,
 	.remove		= omap4_keypad_remove,

-- 
2.30.0

Hi Tony,

On Wed, Jan 06, 2021 at 02:58:22PM +0200, Tony Lindgren wrote:

Simplify probe with devm.

[...]

 	/* input device allocation */
-	keypad_data->input = input_dev = input_allocate_device();
+	keypad_data->input = input_dev = devm_input_allocate_device(&pdev->dev);
 	if (!input_dev) {
 		error = -ENOMEM;
 		goto err_pm_disable;

[...]

 static int omap4_keypad_remove(struct platform_device *pdev)
 {
 	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
-	struct resource *res;
 
 	dev_pm_clear_wake_irq(&pdev->dev);
-
-	free_irq(keypad_data->irq, keypad_data);
-
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-
 	input_unregister_device(keypad_data->input);

not needed:

 * devm_input_allocate_device - allocate managed input device
 * @dev: device owning the input device being created
 *
 * Returns prepared struct input_dev or %NULL.
 *
 * Managed input devices do not need to be explicitly unregistered or
 * freed as it will be done automatically when owner device unbinds from
 * its driver (or binding fails). [...]

-- Sebastian

Hi Tony,

I love your patch! Perhaps something to improve:

[auto build test WARNING on input/next]
[also build test WARNING on hid/for-next linux/master linus/master v5.11-rc2 next-20210104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: openrisc-randconfig-s031-20210106 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-208-g46a52ca4-dirty
        # https://github.com/0day-ci/linux/commit/6a18714b171daeb44c0418e33cc3e78c7daaa44b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
        git checkout 6a18714b171daeb44c0418e33cc3e78c7daaa44b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

   drivers/input/keyboard/omap4-keypad.c: In function 'omap4_keypad_irq_thread_fn':
   drivers/input/keyboard/omap4-keypad.c:161:15: warning: variable 'keys_down' set but not used [-Wunused-but-set-variable]
     161 |  int keys_up, keys_down;
         |               ^~~~~~~~~

quoted

drivers/input/keyboard/omap4-keypad.c:161:6: warning: variable 'keys_up' set but not used [-Wunused-but-set-variable]

     161 |  int keys_up, keys_down;
         |      ^~~~~~~


vim +/keys_up +161 drivers/input/keyboard/omap4-keypad.c

   156	
   157	static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
   158	{
   159		struct omap4_keypad *keypad_data = dev_id;
   160		struct input_dev *input_dev = keypad_data->input;
 > 161		int keys_up, keys_down;
   162		u32 low, high;
   163		u64 keys;
   164	
   165		low = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
   166		high = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
   167		keys = low | (u64)high << 32;
   168	
   169		/* Scan for key up events for lost key-up interrupts */
   170		keys_up = omap4_keypad_scan_state(keypad_data, keys, false);
   171	
   172		/* Scan for key down events */
   173		keys_down = omap4_keypad_scan_state(keypad_data, keys, true);
   174	
   175		input_sync(input_dev);
   176	
   177		keypad_data->keys = keys;
   178	
   179		/* clear pending interrupts */
   180		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
   181				 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
   182	
   183		return IRQ_HANDLED;
   184	}
   185	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Hi Tony,

I love your patch! Perhaps something to improve:

[auto build test WARNING on input/next]
[also build test WARNING on linus/master v5.11-rc2 next-20210104]
[cannot apply to hid/for-next linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: openrisc-randconfig-s031-20210106 (attached as .config)
compiler: or1k-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-208-g46a52ca4-dirty
        # https://github.com/0day-ci/linux/commit/69f44d8d3d1568dd3f330a46f6173a1bfc372ebd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
        git checkout 69f44d8d3d1568dd3f330a46f6173a1bfc372ebd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

   drivers/input/keyboard/omap4-keypad.c: In function 'omap4_keypad_scan_keys':
   drivers/input/keyboard/omap4-keypad.c:162:6: warning: variable 'keys_up' set but not used [-Wunused-but-set-variable]
     162 |  int keys_up, keys_down;
         |      ^~~~~~~
   drivers/input/keyboard/omap4-keypad.c: In function 'omap4_keypad_irq_thread_fn':

quoted

drivers/input/keyboard/omap4-keypad.c:192:7: warning: variable 'down_events' set but not used [-Wunused-but-set-variable]

     192 |  bool down_events;
         |       ^~~~~~~~~~~
   At top level:
   drivers/input/keyboard/omap4-keypad.c:536:32: warning: 'omap4_keypad_pm_ops' defined but not used [-Wunused-const-variable=]
     536 | static const struct dev_pm_ops omap4_keypad_pm_ops = {
         |                                ^~~~~~~~~~~~~~~~~~~


vim +/down_events +192 drivers/input/keyboard/omap4-keypad.c

   158	
   159	static bool omap4_keypad_scan_keys(struct omap4_keypad *keypad_data, bool clear)
   160	{
   161		struct input_dev *input_dev = keypad_data->input;
 > 162		int keys_up, keys_down;
   163		u32 low, high;
   164		u64 keys = 0;
   165	
   166		mutex_lock(&keypad_data->lock);
   167		if (!clear) {
   168			low = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
   169			high = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
   170			keys = low | (u64)high << 32;
   171		}
   172	
   173		/* Scan for key up events for lost key-up interrupts */
   174		keys_up = omap4_keypad_scan_state(keypad_data, keys, false);
   175	
   176		/* Scan for key down events */
   177		keys_down = omap4_keypad_scan_state(keypad_data, keys, true);
   178	
   179		input_sync(input_dev);
   180	
   181		keypad_data->keys = keys;
   182	
   183		mutex_unlock(&keypad_data->lock);
   184	
   185		return keys_down;
   186	}
   187	
   188	static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
   189	{
   190		struct omap4_keypad *keypad_data = dev_id;
   191		struct device *dev = keypad_data->input->dev.parent;
 > 192		bool down_events;
   193		int error;
   194	
   195		error = pm_runtime_get_sync(dev);
   196		if (error < 0) {
   197			pm_runtime_put_noidle(dev);
   198	
   199			return IRQ_NONE;
   200		}
   201	
   202		down_events = omap4_keypad_scan_keys(keypad_data, false);
   203	
   204		/* clear pending interrupts */
   205		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
   206				 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
   207	
   208		pm_runtime_mark_last_busy(dev);
   209		pm_runtime_put_autosuspend(dev);
   210	
   211		return IRQ_HANDLED;
   212	}
   213	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Hi Tony,

I love your patch! Perhaps something to improve:

[auto build test WARNING on input/next]
[also build test WARNING on linus/master v5.11-rc2 next-20210104]
[cannot apply to hid/for-next linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: x86_64-randconfig-a014-20210106 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/69f44d8d3d1568dd3f330a46f6173a1bfc372ebd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
        git checkout 69f44d8d3d1568dd3f330a46f6173a1bfc372ebd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

quoted

drivers/input/keyboard/omap4-keypad.c:536:32: warning: unused variable 'omap4_keypad_pm_ops' [-Wunused-const-variable]

   static const struct dev_pm_ops omap4_keypad_pm_ops = {
                                  ^
   1 warning generated.


vim +/omap4_keypad_pm_ops +536 drivers/input/keyboard/omap4-keypad.c

   535	
 > 536	static const struct dev_pm_ops omap4_keypad_pm_ops = {
   537		SET_RUNTIME_PM_OPS(omap4_keypad_runtime_suspend, NULL, NULL)
   538	};
   539	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Hi Tony,

I love your patch! Perhaps something to improve:

[auto build test WARNING on input/next]
[also build test WARNING on linus/master v5.11-rc2 next-20210104]
[cannot apply to hid/for-next linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: s390-randconfig-r024-20210106 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/69f44d8d3d1568dd3f330a46f6173a1bfc372ebd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
        git checkout 69f44d8d3d1568dd3f330a46f6173a1bfc372ebd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x000000ffUL) << 24) |            \
                     ^
   In file included from drivers/input/keyboard/omap4-keypad.c:15:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x0000ff00UL) <<  8) |            \
                     ^
   In file included from drivers/input/keyboard/omap4-keypad.c:15:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x00ff0000UL) >>  8) |            \
                     ^
   In file included from drivers/input/keyboard/omap4-keypad.c:15:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0xff000000UL) >> 24)))
                     ^
   In file included from drivers/input/keyboard/omap4-keypad.c:15:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
           __fswab32(x))
                     ^
   In file included from drivers/input/keyboard/omap4-keypad.c:15:
   In file included from include/linux/io.h:13:
   In file included from arch/s390/include/asm/io.h:72:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^

quoted

drivers/input/keyboard/omap4-keypad.c:536:32: warning: unused variable 'omap4_keypad_pm_ops' [-Wunused-const-variable]

   static const struct dev_pm_ops omap4_keypad_pm_ops = {
                                  ^
   21 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MFD_SYSCON
   Depends on HAS_IOMEM
   Selected by
   - MTD_NAND_STM32_FMC2 && MTD && MTD_RAW_NAND && (MACH_STM32MP157 || COMPILE_TEST
   - MTD_NAND_MESON && MTD && MTD_RAW_NAND && (ARCH_MESON || COMPILE_TEST
   - POWER_RESET_OCELOT_RESET && POWER_RESET && (MSCC_OCELOT || COMPILE_TEST
   - INGENIC_TCU_IRQ && (MIPS || COMPILE_TEST
   - PHY_BCM_SR_PCIE && OF && (ARCH_BCM_IPROC || COMPILE_TEST
   - PHY_HI3660_USB && (ARCH_HISI && ARM64 || COMPILE_TEST
   - PHY_HISTB_COMBPHY && (ARCH_HISI && ARM64 || COMPILE_TEST
   - PHY_DA8XX_USB && (ARCH_DAVINCI_DA8XX || COMPILE_TEST
   WARNING: unmet direct dependencies detected for MFD_STM32_TIMERS
   Depends on HAS_IOMEM && (ARCH_STM32 && OF || COMPILE_TEST
   Selected by
   - STM32_ADC_CORE && IIO && (ARCH_STM32 || COMPILE_TEST && OF && REGULATOR


vim +/omap4_keypad_pm_ops +536 drivers/input/keyboard/omap4-keypad.c

   535	
 > 536	static const struct dev_pm_ops omap4_keypad_pm_ops = {
   537		SET_RUNTIME_PM_OPS(omap4_keypad_runtime_suspend, NULL, NULL)
   538	};
   539	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Hi Tony,

I love your patch! Yet something to improve:

[auto build test ERROR on input/next]
[also build test ERROR on linus/master v5.11-rc2 next-20210104]
[cannot apply to hid/for-next linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: s390-randconfig-r024-20210106 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 5c951623bc8965fa1e89660f2f5f4a2944e4981a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/f42e31515b38ac05424768b08a12316f301cfd1a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tony-Lindgren/Lost-key-up-interrupt-handling-for-omap4-keypad/20210106-210045
        git checkout f42e31515b38ac05424768b08a12316f301cfd1a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All errors (new ones prefixed by >>):

   dma-crossbar.c:(.text+0x344): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: dma-crossbar.c:(.text+0x48a): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/dma/xilinx/xilinx_dpdma.o: in function `xilinx_dpdma_probe':
   xilinx_dpdma.c:(.text+0x6e): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/soc/aspeed/aspeed-lpc-ctrl.o: in function `aspeed_lpc_ctrl_probe':
   aspeed-lpc-ctrl.c:(.text+0x72): undefined reference to `of_address_to_resource'
   s390x-linux-gnu-ld: aspeed-lpc-ctrl.c:(.text+0xd6): undefined reference to `of_address_to_resource'
   s390x-linux-gnu-ld: drivers/soc/fsl/dpaa2-console.o: in function `dpaa2_console_probe':
   dpaa2-console.c:(.text+0x30): undefined reference to `of_address_to_resource'
   s390x-linux-gnu-ld: drivers/soc/fsl/dpaa2-console.o: in function `dpaa2_mc_console_open':
   dpaa2-console.c:(.text+0x34e): undefined reference to `ioremap'
   s390x-linux-gnu-ld: dpaa2-console.c:(.text+0x37c): undefined reference to `iounmap'
   s390x-linux-gnu-ld: dpaa2-console.c:(.text+0x394): undefined reference to `ioremap'
   s390x-linux-gnu-ld: dpaa2-console.c:(.text+0x498): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/soc/fsl/dpaa2-console.o: in function `dpaa2_console_close':
   dpaa2-console.c:(.text+0x4e8): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/soc/fsl/dpaa2-console.o: in function `dpaa2_aiop_console_open':
   dpaa2-console.c:(.text+0x56e): undefined reference to `ioremap'
   s390x-linux-gnu-ld: dpaa2-console.c:(.text+0x59c): undefined reference to `iounmap'
   s390x-linux-gnu-ld: dpaa2-console.c:(.text+0x5b4): undefined reference to `ioremap'
   s390x-linux-gnu-ld: dpaa2-console.c:(.text+0x6b2): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/soc/imx/soc-imx8m.o: in function `imx8mq_soc_revision':
   soc-imx8m.c:(.init.text+0x1f0): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: soc-imx8m.c:(.init.text+0x232): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/soc/imx/soc-imx8m.o: in function `imx8mm_soc_revision':
   soc-imx8m.c:(.init.text+0x2aa): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: soc-imx8m.c:(.init.text+0x2bc): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/soc/imx/soc-imx8m.o: in function `imx8mm_soc_uid':
   soc-imx8m.c:(.init.text+0x33a): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: soc-imx8m.c:(.init.text+0x366): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/soc/mediatek/mtk-pmic-wrap.o: in function `pwrap_probe':
   mtk-pmic-wrap.c:(.text+0xb0): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: mtk-pmic-wrap.c:(.text+0x140): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/soc/amlogic/meson-canvas.o: in function `meson_canvas_probe':
   meson-canvas.c:(.text+0x42a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/soc/amlogic/meson-clk-measure.o: in function `meson_msr_probe':
   meson-clk-measure.c:(.text+0x8e): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/soc/qcom/qcom-geni-se.o: in function `geni_se_probe':
   qcom-geni-se.c:(.text+0xc90): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/soc/qcom/llcc-qcom.o:llcc-qcom.c:(.text+0x69c): more undefined references to `devm_ioremap_resource' follow
   s390x-linux-gnu-ld: drivers/soc/renesas/renesas-soc.o: in function `renesas_soc_init':
   renesas-soc.c:(.init.text+0x70): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: renesas-soc.c:(.init.text+0x9e): undefined reference to `iounmap'
   s390x-linux-gnu-ld: renesas-soc.c:(.init.text+0xe6): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: renesas-soc.c:(.init.text+0x10a): undefined reference to `iounmap'
   s390x-linux-gnu-ld: renesas-soc.c:(.init.text+0x18a): undefined reference to `ioremap'
   s390x-linux-gnu-ld: drivers/soc/renesas/rcar-rst.o: in function `rcar_rst_init':
   rcar-rst.c:(.init.text+0xa8): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: drivers/soc/renesas/rcar-sysc.o: in function `rcar_sysc_pd_init':
   rcar-sysc.c:(.init.text+0xa6): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: drivers/regulator/stm32-vrefbuf.o: in function `stm32_vrefbuf_probe':
   stm32-vrefbuf.c:(.text+0x5a): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/reset/reset-ath79.o: in function `ath79_reset_probe':
   reset-ath79.c:(.text+0x60): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/reset/reset-brcmstb.o: in function `brcmstb_reset_probe':
   reset-brcmstb.c:(.text+0x5e): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/reset/reset-lpc18xx.o: in function `lpc18xx_rgu_probe':
   reset-lpc18xx.c:(.text+0x5a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/reset/reset-qcom-aoss.o: in function `qcom_aoss_reset_probe':
   reset-qcom-aoss.c:(.text+0x74): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/reset/reset-qcom-pdc.o: in function `qcom_pdc_reset_probe':
   reset-qcom-pdc.c:(.text+0x5a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/reset/reset-simple.o:reset-simple.c:(.text+0x310): more undefined references to `devm_ioremap_resource' follow
   s390x-linux-gnu-ld: drivers/char/hw_random/exynos-trng.o: in function `exynos_trng_probe':
   exynos-trng.c:(.text+0xa0): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/char/hw_random/meson-rng.o: in function `meson_rng_probe':
   meson-rng.c:(.text+0x4e): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/char/hw_random/mtk-rng.o: in function `mtk_rng_probe':
   mtk-rng.c:(.text+0x84): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/char/hw_random/ks-sa-rng.o: in function `ks_sa_rng_probe':
   ks-sa-rng.c:(.text+0x96): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/char/hw_random/npcm-rng.o: in function `npcm_rng_probe':
   npcm-rng.c:(.text+0x48): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/char/xillybus/xillybus_of.o:xillybus_of.c:(.text+0x74): more undefined references to `devm_platform_ioremap_resource' follow
   s390x-linux-gnu-ld: drivers/mfd/syscon.o: in function `device_node_get_regmap':
   syscon.c:(.text+0x108): undefined reference to `of_address_to_resource'
   s390x-linux-gnu-ld: syscon.c:(.text+0x162): undefined reference to `ioremap'
   s390x-linux-gnu-ld: syscon.c:(.text+0x314): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/mfd/syscon.o: in function `syscon_probe':
   syscon.c:(.text+0x5d0): undefined reference to `devm_ioremap'
   s390x-linux-gnu-ld: drivers/mfd/stm32-timers.o: in function `stm32_timers_probe':
   stm32-timers.c:(.text+0x4de): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/mtd/nand/raw/mxic_nand.o: in function `mxic_nfc_probe':
   mxic_nand.c:(.text+0x54): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/mtd/nand/raw/stm32_fmc2_nand.o: in function `stm32_fmc2_nfc_probe':
   stm32_fmc2_nand.c:(.text+0x2bc): undefined reference to `of_address_to_resource'
   s390x-linux-gnu-ld: stm32_fmc2_nand.c:(.text+0x4a2): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: stm32_fmc2_nand.c:(.text+0x4ee): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: stm32_fmc2_nand.c:(.text+0x52e): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: stm32_fmc2_nand.c:(.text+0x570): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: stm32_fmc2_nand.c:(.text+0x5b6): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/mtd/nand/raw/stm32_fmc2_nand.o:stm32_fmc2_nand.c:(.text+0x5ea): more undefined references to `devm_ioremap_resource' follow
   s390x-linux-gnu-ld: drivers/input/serio/sun4i-ps2.o: in function `sun4i_ps2_probe':
   sun4i-ps2.c:(.text+0xb4): undefined reference to `ioremap'
   s390x-linux-gnu-ld: sun4i-ps2.c:(.text+0x230): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/input/serio/sun4i-ps2.o: in function `sun4i_ps2_remove':
   sun4i-ps2.c:(.text+0x2b8): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/input/keyboard/goldfish_events.o: in function `events_probe':
   goldfish_events.c:(.text+0x62): undefined reference to `devm_ioremap'
   s390x-linux-gnu-ld: drivers/input/keyboard/omap4-keypad.o: in function `omap4_keypad_probe':

quoted

omap4-keypad.c:(.text+0x15c): undefined reference to `devm_ioremap'

   s390x-linux-gnu-ld: drivers/input/keyboard/st-keyscan.o: in function `keyscan_probe':
   st-keyscan.c:(.text+0x136): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/power/reset/ocelot-reset.o: in function `ocelot_reset_probe':
   ocelot-reset.c:(.text+0x5a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/crypto/atmel-sha.o: in function `atmel_sha_probe':
   atmel-sha.c:(.text+0x17a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/crypto/atmel-tdes.o: in function `atmel_tdes_probe':
   atmel-tdes.c:(.text+0x17a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/crypto/ccree/cc_driver.o: in function `ccree_probe':
   cc_driver.c:(.text+0x36c): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/crypto/ccree/cc_debugfs.o: in function `cc_debugfs_init':
   cc_debugfs.c:(.text+0x94): undefined reference to `debugfs_create_regset32'
   s390x-linux-gnu-ld: cc_debugfs.c:(.text+0x13a): undefined reference to `debugfs_create_regset32'
   s390x-linux-gnu-ld: drivers/crypto/qcom-rng.o: in function `qcom_rng_probe':
   qcom-rng.c:(.text+0x60): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/clocksource/timer-of.o: in function `timer_of_init':
   timer-of.c:(.init.text+0x50): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: timer-of.c:(.init.text+0xec): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/clocksource/timer-of.o: in function `timer_of_cleanup':
   timer-of.c:(.init.text+0x2e6): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/clocksource/timer-davinci.o: in function `davinci_timer_register':
   timer-davinci.c:(.init.text+0x78): undefined reference to `ioremap'
   s390x-linux-gnu-ld: drivers/clocksource/timer-davinci.o: in function `of_davinci_timer_register':
   timer-davinci.c:(.init.text+0x2ca): undefined reference to `of_address_to_resource'
   s390x-linux-gnu-ld: drivers/clocksource/mxs_timer.o: in function `mxs_timer_init':
   mxs_timer.c:(.init.text+0x20): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: drivers/clocksource/timer-zevio.o: in function `zevio_timer_add':
   timer-zevio.c:(.init.text+0x58): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: timer-zevio.c:(.init.text+0xb8): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/clocksource/armv7m_systick.o: in function `system_timer_of_register':
   armv7m_systick.c:(.init.text+0x2e): undefined reference to `of_iomap'
   s390x-linux-gnu-ld: armv7m_systick.c:(.init.text+0xe2): undefined reference to `iounmap'
   s390x-linux-gnu-ld: drivers/mailbox/armada-37xx-rwtm-mailbox.o: in function `armada_37xx_mbox_probe':
   armada-37xx-rwtm-mailbox.c:(.text+0x68): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/mailbox/qcom-apcs-ipc-mailbox.o: in function `qcom_apcs_ipc_probe':
   qcom-apcs-ipc-mailbox.c:(.text+0x5a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/mailbox/mtk-cmdq-mailbox.o: in function `cmdq_probe':
   mtk-cmdq-mailbox.c:(.text+0x7e): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/mailbox/sprd-mailbox.o: in function `sprd_mbox_probe':
   sprd-mailbox.c:(.text+0x4e): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: sprd-mailbox.c:(.text+0x6c): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/iio/adc/adi-axi-adc.o: in function `adi_axi_adc_probe':
   adi-axi-adc.c:(.text+0x386): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/iio/adc/at91_adc.o: in function `at91_adc_probe':
   at91_adc.c:(.text+0x112): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/iio/adc/ingenic-adc.o: in function `ingenic_adc_probe':
   ingenic-adc.c:(.text+0xea): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/iio/adc/rcar-gyroadc.o:rcar-gyroadc.c:(.text+0x50): more undefined references to `devm_platform_ioremap_resource' follow
   s390x-linux-gnu-ld: drivers/iio/adc/stm32-adc-core.o: in function `stm32_adc_probe':
   stm32-adc-core.c:(.text+0x9e): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/iio/adc/stm32-dfsdm-core.o: in function `stm32_dfsdm_probe':
   stm32-dfsdm-core.c:(.text+0x29a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/iio/dac/stm32-dac-core.o: in function `stm32_dac_probe':
   stm32-dac-core.c:(.text+0x8a): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/nvmem/imx-iim.o: in function `imx_iim_probe':
   imx-iim.c:(.text+0x4e): undefined reference to `devm_platform_ioremap_resource'
   s390x-linux-gnu-ld: drivers/nvmem/meson-mx-efuse.o: in function `meson_mx_efuse_probe':
   meson-mx-efuse.c:(.text+0x6e): undefined reference to `devm_ioremap_resource'
   s390x-linux-gnu-ld: drivers/counter/ti-eqep.o: in function `ti_eqep_probe':
   ti-eqep.c:(.text+0x48): undefined reference to `devm_platform_ioremap_resource'

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for MFD_SYSCON
   Depends on HAS_IOMEM
   Selected by
   - MTD_NAND_STM32_FMC2 && MTD && MTD_RAW_NAND && (MACH_STM32MP157 || COMPILE_TEST
   - MTD_NAND_MESON && MTD && MTD_RAW_NAND && (ARCH_MESON || COMPILE_TEST
   - POWER_RESET_OCELOT_RESET && POWER_RESET && (MSCC_OCELOT || COMPILE_TEST
   - INGENIC_TCU_IRQ && (MIPS || COMPILE_TEST
   - PHY_BCM_SR_PCIE && OF && (ARCH_BCM_IPROC || COMPILE_TEST
   - PHY_HI3660_USB && (ARCH_HISI && ARM64 || COMPILE_TEST
   - PHY_HISTB_COMBPHY && (ARCH_HISI && ARM64 || COMPILE_TEST
   - PHY_DA8XX_USB && (ARCH_DAVINCI_DA8XX || COMPILE_TEST
   WARNING: unmet direct dependencies detected for MFD_STM32_TIMERS
   Depends on HAS_IOMEM && (ARCH_STM32 && OF || COMPILE_TEST
   Selected by
   - STM32_ADC_CORE && IIO && (ARCH_STM32 || COMPILE_TEST && OF && REGULATOR

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

* Sebastian Reichel [off-list ref] [210106 13:44]:

On Wed, Jan 06, 2021 at 02:58:22PM +0200, Tony Lindgren wrote:

quoted

-	struct resource *res;
 
 	dev_pm_clear_wake_irq(&pdev->dev);
-
-	free_irq(keypad_data->irq, keypad_data);
-
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-
 	input_unregister_device(keypad_data->input);

not needed:

 * devm_input_allocate_device - allocate managed input device
 * @dev: device owning the input device being created
 *
 * Returns prepared struct input_dev or %NULL.
 *
 * Managed input devices do not need to be explicitly unregistered or
 * freed as it will be done automatically when owner device unbinds from
 * its driver (or binding fails). [...]

OK thanks will drop and fix up the reported autobuild warnings
and repost. Looks like also the OMAP4_KEYPAD_AUTOIDLE_MS value
of 50 is too long, I recently changed it from 30 but now have
seen few stuck last pressed keys.

Regards,

Tony

Hi Tony,

On Wed, Jan 06, 2021 at 02:58:22PM +0200, Tony Lindgren wrote:

 static int omap4_keypad_remove(struct platform_device *pdev)
 {
 	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
-	struct resource *res;
 
 	dev_pm_clear_wake_irq(&pdev->dev);
-
-	free_irq(keypad_data->irq, keypad_data);
-
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);

I do not quite like that we need to keep this in remove(). I had the
patch below for quite some time, could you please try it?

Thanks!

-- 
Dmitry


Input: omap4-keypad - switch to use managed resources

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Now that input core supports devres-managed input devices we can clean
up this driver a bit.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/omap4-keypad.c |  139 +++++++++++++--------------------
 1 file changed, 55 insertions(+), 84 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index b17ac2a295b9..d36774a55a10 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c

@@ -252,8 +252,14 @@ static int omap4_keypad_check_revision(struct device *dev,
 	return 0;
 }
 
+static void omap4_disable_pm(void *d)
+{
+	pm_runtime_disable(d);
+}
+
 static int omap4_keypad_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct omap4_keypad *keypad_data;
 	struct input_dev *input_dev;
 	struct resource *res;

@@ -271,33 +277,30 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	keypad_data = kzalloc(sizeof(struct omap4_keypad), GFP_KERNEL);
+	keypad_data = devm_kzalloc(dev, sizeof(struct omap4_keypad),
+				   GFP_KERNEL);
 	if (!keypad_data) {
-		dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
+		dev_err(dev, "keypad_data memory allocation failed\n");
 		return -ENOMEM;
 	}
 
 	keypad_data->irq = irq;
 
-	error = omap4_keypad_parse_dt(&pdev->dev, keypad_data);
+	error = omap4_keypad_parse_dt(dev, keypad_data);
 	if (error)
-		goto err_free_keypad;
+		return error;
 
-	res = request_mem_region(res->start, resource_size(res), pdev->name);
-	if (!res) {
-		dev_err(&pdev->dev, "can't request mem region\n");
-		error = -EBUSY;
-		goto err_free_keypad;
-	}
+	keypad_data->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(keypad_data->base))
+		return PTR_ERR(keypad_data->base);
 
-	keypad_data->base = ioremap(res->start, resource_size(res));
-	if (!keypad_data->base) {
-		dev_err(&pdev->dev, "can't ioremap mem resource\n");
-		error = -ENOMEM;
-		goto err_release_mem;
-	}
+	pm_runtime_enable(dev);
 
-	pm_runtime_enable(&pdev->dev);
+	error = devm_add_action_or_reset(dev, omap4_disable_pm, dev);
+	if (error) {
+		dev_err(dev, "unable to register cleanup action\n");
+		return error;
+	}
 
 	/*
 	 * Enable clocks for the keypad module so that we can read

@@ -307,27 +310,26 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	if (error) {
 		dev_err(&pdev->dev, "pm_runtime_get_sync() failed\n");
 		pm_runtime_put_noidle(&pdev->dev);
-	} else {
-		error = omap4_keypad_check_revision(&pdev->dev,
-						    keypad_data);
-		if (!error) {
-			/* Ensure device does not raise interrupts */
-			omap4_keypad_stop(keypad_data);
-		}
-		pm_runtime_put_sync(&pdev->dev);
+		return error;
+	}
+
+	error = omap4_keypad_check_revision(&pdev->dev,
+					    keypad_data);
+	if (!error) {
+		/* Ensure device does not raise interrupts */
+		omap4_keypad_stop(keypad_data);
 	}
+
+	pm_runtime_put_sync(&pdev->dev);
 	if (error)
-		goto err_pm_disable;
+		return error;
 
 	/* input device allocation */
-	keypad_data->input = input_dev = input_allocate_device();
-	if (!input_dev) {
-		error = -ENOMEM;
-		goto err_pm_disable;
-	}
+	keypad_data->input = input_dev = devm_input_allocate_device(dev);
+	if (!input_dev)
+		return -ENOMEM;
 
 	input_dev->name = pdev->name;
-	input_dev->dev.parent = &pdev->dev;
 	input_dev->id.bustype = BUS_HOST;
 	input_dev->id.vendor = 0x0001;
 	input_dev->id.product = 0x0001;

@@ -344,84 +346,53 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 
 	keypad_data->row_shift = get_count_order(keypad_data->cols);
 	max_keys = keypad_data->rows << keypad_data->row_shift;
-	keypad_data->keymap = kcalloc(max_keys,
-				      sizeof(keypad_data->keymap[0]),
-				      GFP_KERNEL);
+	keypad_data->keymap = devm_kcalloc(dev,
+					   max_keys,
+					   sizeof(keypad_data->keymap[0]),
+					   GFP_KERNEL);
 	if (!keypad_data->keymap) {
-		dev_err(&pdev->dev, "Not enough memory for keymap\n");
-		error = -ENOMEM;
-		goto err_free_input;
+		dev_err(dev, "Not enough memory for keymap\n");
+		return -ENOMEM;
 	}
 
 	error = matrix_keypad_build_keymap(NULL, NULL,
 					   keypad_data->rows, keypad_data->cols,
 					   keypad_data->keymap, input_dev);
 	if (error) {
-		dev_err(&pdev->dev, "failed to build keymap\n");
-		goto err_free_keymap;
+		dev_err(dev, "failed to build keymap\n");
+		return error;
 	}
 
-	error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
-				     omap4_keypad_irq_thread_fn, IRQF_ONESHOT,
-				     "omap4-keypad", keypad_data);
+	error = devm_request_threaded_irq(dev, keypad_data->irq,
+					  omap4_keypad_irq_handler,
+					  omap4_keypad_irq_thread_fn,
+					  IRQF_ONESHOT,
+					  "omap4-keypad", keypad_data);
 	if (error) {
-		dev_err(&pdev->dev, "failed to register interrupt\n");
-		goto err_free_keymap;
+		dev_err(dev, "failed to register interrupt\n");
+		return error;
 	}
 
 	error = input_register_device(keypad_data->input);
-	if (error < 0) {
-		dev_err(&pdev->dev, "failed to register input device\n");
-		goto err_free_irq;
+	if (error) {
+		dev_err(dev, "failed to register input device\n");
+		return error;
 	}
 
-	device_init_wakeup(&pdev->dev, true);
-	error = dev_pm_set_wake_irq(&pdev->dev, keypad_data->irq);
+	device_init_wakeup(dev, true);
+	error = dev_pm_set_wake_irq(dev, keypad_data->irq);
 	if (error)
-		dev_warn(&pdev->dev,
-			 "failed to set up wakeup irq: %d\n", error);
+		dev_warn(dev, "failed to set up wakeup irq: %d\n", error);
 
 	platform_set_drvdata(pdev, keypad_data);
 
 	return 0;
-
-err_free_irq:
-	free_irq(keypad_data->irq, keypad_data);
-err_free_keymap:
-	kfree(keypad_data->keymap);
-err_free_input:
-	input_free_device(input_dev);
-err_pm_disable:
-	pm_runtime_disable(&pdev->dev);
-	iounmap(keypad_data->base);
-err_release_mem:
-	release_mem_region(res->start, resource_size(res));
-err_free_keypad:
-	kfree(keypad_data);
-	return error;
 }
 
 static int omap4_keypad_remove(struct platform_device *pdev)
 {
-	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
-	struct resource *res;
-
 	dev_pm_clear_wake_irq(&pdev->dev);
 
-	free_irq(keypad_data->irq, keypad_data);
-
-	pm_runtime_disable(&pdev->dev);
-
-	input_unregister_device(keypad_data->input);
-
-	iounmap(keypad_data->base);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-
-	kfree(keypad_data->keymap);
-	kfree(keypad_data);
-
 	return 0;
 }
 

Hi Tony,

On Wed, Jan 06, 2021 at 02:58:21PM +0200, Tony Lindgren wrote:

This, and corresponding changes in open() and close() seem like a
separate improvement. Do you mind splitting them into a separate patch,
and have the missing key release fix go on top of it?

Thanks.

-- 
Dmitry

* Dmitry Torokhov [off-list ref] [210110 06:31]:

I do not quite like that we need to keep this in remove(). I had the
patch below for quite some time, could you please try it?

Yes seems to work nice :)

Input: omap4-keypad - switch to use managed resources

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Now that input core supports devres-managed input devices we can clean
up this driver a bit.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Tested-by: Tony Lindgren <tony@atomide.com>

* Dmitry Torokhov [off-list ref] [210110 06:34]:

Hi Tony,

On Wed, Jan 06, 2021 at 02:58:21PM +0200, Tony Lindgren wrote:

quoted

@@ -301,6 +348,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	}
 
 	keypad_data->irq = irq;
+	mutex_init(&keypad_data->lock);
 
 	error = omap4_keypad_parse_dt(&pdev->dev, keypad_data);
 	if (error)

@@ -320,6 +368,8 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 		goto err_release_mem;
 	}
 
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, OMAP4_KEYPAD_AUTOIDLE_MS);

This, and corresponding changes in open() and close() seem like a
separate improvement. Do you mind splitting them into a separate patch,
and have the missing key release fix go on top of it?

Sure will do.

Thanks,

Tony