[PATCH 1/4] Input: fix poller start order on uninhibit

[PATCH 1/4] Input: fix poller start order on uninhibit

[Dmitry Torokhov]

When uninhibiting a device, we start the poller before marking the
device as uninhibited (setting dev->inhibited = false). Since the
poller immediately polls the device and reports events via input_event(),
these initial events are dropped because dev->inhibited is still true.

Fix this by starting the poller only after the device is fully uninhibited
and its state is replayed.

Fixes: a181616487db ("Input: Add "inhibited" property")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index cf6fecea79b8..d99dfe69e12d 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1793,8 +1793,6 @@ static int input_uninhibit_device(struct input_dev *dev)
 			if (error)
 				return error;
 		}
-		if (dev->poller)
-			input_dev_poller_start(dev->poller);
 	}
 
 	dev->inhibited = false;
@@ -1802,6 +1800,9 @@ static int input_uninhibit_device(struct input_dev *dev)
 	scoped_guard(spinlock_irq, &dev->event_lock)
 		input_dev_toggle(dev, true);
 
+	if (dev->users && dev->poller)
+		input_dev_poller_start(dev->poller);
+
 	return 0;
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 2/4] Input: call handler->start() when uninhibiting device

[Dmitry Torokhov]

When an input device is inhibited via input_inhibit_device(), the driver
is closed and physical feedback (like LEDs and sounds) is toggled off.
However, from the input core's perspective, the handles remain open.

When the device is later uninhibited, the driver is re-opened. While the
core restores simple LED states via input_dev_toggle(), complex handlers
(such as vt/keyboard) may need to re-synchronize their broader logical
state with the hardware.

Fixes: a181616487db ("Input: Add "inhibited" property")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index d99dfe69e12d..c2a038d31beb 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1780,6 +1780,7 @@ static int input_inhibit_device(struct input_dev *dev)
 
 static int input_uninhibit_device(struct input_dev *dev)
 {
+	struct input_handle *handle;
 	int error;
 
 	guard(mutex)(&dev->mutex);
@@ -1803,6 +1804,11 @@ static int input_uninhibit_device(struct input_dev *dev)
 	if (dev->users && dev->poller)
 		input_dev_poller_start(dev->poller);
 
+	list_for_each_entry(handle, &dev->h_list, d_node) {
+		if (handle->open && handle->handler->start)
+			handle->handler->start(handle);
+	}
+
 	return 0;
 }
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 3/4] Input: defer handler's start() until device is opened

[Dmitry Torokhov]

When registering an input handle, handler->start() is currently called
immediately. However, the input device might not be fully opened or
ready to process events at this stage, meaning any state synchronization
events (like setting LED states) injected by the handler's start method
might be dropped.

Move the handler->start() invocation to input_open_device(). If it is
the first handle opening the device, start() is called after the driver's
open() method has successfully completed and the device is fully prepared.

To facilitate this, factor out the device startup logic (calling driver's
open and starting polling) into input_start_device().

For passive observer handlers, their start() method is also deferred
until the handle is opened. Since opening a passive observer handle does
not start the underlying hardware device, their start() method is called
immediately upon opening, regardless of whether the device is active.

Fixes: c7e8dc6ee6d5 ("Input: add start() method to input handlers")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 45 +++++++++++++++++++++++++------------------
 include/linux/input.h |  5 +++--
 2 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index c2a038d31beb..0a95cbdc467e 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -568,6 +568,28 @@ void input_release_device(struct input_handle *handle)
 }
 EXPORT_SYMBOL(input_release_device);
 
+static int input_start_device(struct input_dev *dev)
+{
+	int error;
+
+	lockdep_assert_held(&dev->mutex);
+
+	if (dev->users++ == 0 && !dev->inhibited) {
+		if (dev->open) {
+			error = dev->open(dev);
+			if (error) {
+				dev->users--;
+				return error;
+			}
+		}
+
+		if (dev->poller)
+			input_dev_poller_start(dev->poller);
+	}
+
+	return 0;
+}
+
 /**
  * input_open_device - open input device
  * @handle: handle through which device is being accessed
@@ -586,21 +608,9 @@ int input_open_device(struct input_handle *handle)
 
 		handle->open++;
 
-		if (handle->handler->passive_observer)
-			return 0;
-
-		if (dev->users++ || dev->inhibited) {
-			/*
-			 * Device is already opened and/or inhibited,
-			 * so we can exit immediately and report success.
-			 */
-			return 0;
-		}
-
-		if (dev->open) {
-			error = dev->open(dev);
+		if (!handle->handler->passive_observer) {
+			error = input_start_device(dev);
 			if (error) {
-				dev->users--;
 				handle->open--;
 				/*
 				 * Make sure we are not delivering any more
@@ -611,8 +621,8 @@ int input_open_device(struct input_handle *handle)
 			}
 		}
 
-		if (dev->poller)
-			input_dev_poller_start(dev->poller);
+		if (handle->open == 1 && handle->handler->start)
+			handle->handler->start(handle);
 	}
 
 	return 0;
@@ -2662,9 +2672,6 @@ int input_register_handle(struct input_handle *handle)
 	 */
 	list_add_tail_rcu(&handle->h_node, &handler->h_list);
 
-	if (handler->start)
-		handler->start(handle);
-
 	return 0;
 }
 EXPORT_SYMBOL(input_register_handle);
diff --git a/include/linux/input.h b/include/linux/input.h
index 3022bb730898..f7a2cfad5448 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -284,8 +284,9 @@ struct input_handle;
  * @connect: called when attaching a handler to an input device
  * @disconnect: disconnects a handler from input device
  * @start: starts handler for given handle. This function is called by
- *	input core right after connect() method and also when a process
- *	that "grabbed" a device releases it
+ *	input core when device is open and ready to process events,
+ *	and also when device is uninhibited or when a process that "grabbed"
+ *	a device releases it
  * @passive_observer: set to %true by drivers only interested in observing
  *	data stream from devices if there are other users present. Such
  *	drivers will not result in starting underlying hardware device
-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 4/4] Input: ensure device is ready before delivering events

[Dmitry Torokhov]

When a device is opened via input_open_device(), the driver's open()
callback is invoked. Some drivers, like cm109, submit URBs or perform
other hardware initialization in their open() callbacks.

However, the input core does not prevent dev->event() from being called
concurrently during the driver's open() execution. For instance, if a
console beep occurs, the kbd handler might inject an EV_SND event. This
can lead to double list_add BUGs if the driver submits the same URB in
both open() and event() paths without adequate synchronization.

To fix this, introduce a ready flag in the input_dev structure.
For complex devices (where dev->open is defined), this flag is set to true
only after the driver's open() method successfully completes. The core now
checks ready in input_event_dispose() and input_dev_toggle()
to prevent events from reaching the hardware before it is fully prepared.
For simple devices (no open callback), events are delivered immediately.

We also replay the logical state in input_open_device() by calling
input_dev_toggle() right after marking the device ready, ensuring no
events are permanently lost.

In the inhibit path, we ensure that physical feedback (LEDs/sounds) is
turned off before the device is closed, and we synchronize the inhibited
state transition under the event lock to prevent races with incoming events.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/input.c | 107 ++++++++++++++++++++++++++----------------
 include/linux/input.h |  12 +++--
 2 files changed, 75 insertions(+), 44 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 0a95cbdc467e..724cc146fc09 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -318,7 +318,7 @@ static int input_get_disposition(struct input_dev *dev,
 static void input_event_dispose(struct input_dev *dev, int disposition,
 				unsigned int type, unsigned int code, int value)
 {
-	if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
+	if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event && dev->ready)
 		dev->event(dev, type, code, value);
 
 	if (disposition & INPUT_PASS_TO_HANDLERS) {
@@ -568,6 +568,48 @@ void input_release_device(struct input_handle *handle)
 }
 EXPORT_SYMBOL(input_release_device);
 
+#define INPUT_DO_TOGGLE(dev, type, bits, on)				\
+	do {								\
+		int i;							\
+		bool active;						\
+									\
+		if (!test_bit(EV_##type, dev->evbit))			\
+			break;						\
+									\
+		for_each_set_bit(i, dev->bits##bit, type##_CNT) {	\
+			active = test_bit(i, dev->bits);		\
+			if (!active && !on)				\
+				continue;				\
+									\
+			dev->event(dev, EV_##type, i, on ? active : 0);	\
+		}							\
+	} while (0)
+
+/*
+ * Iterate through the logical state of the input device (LEDs, sounds,
+ * auto-repeat) and explicitly push that state down to the hardware
+ * via dev->event() to match the current logical state (if activate is true),
+ * or forcibly turn off all feedback like LEDs and sounds during teardown
+ * or suspend (if activate is false).
+ *
+ * Primarily used as a state-replay mechanism after a device is opened
+ * or uninhibited, as events might have been dropped by the core while the
+ * hardware was not marked as ready.
+ */
+static void input_dev_toggle(struct input_dev *dev, bool activate)
+{
+	if (!dev->event || !dev->ready)
+		return;
+
+	INPUT_DO_TOGGLE(dev, LED, led, activate);
+	INPUT_DO_TOGGLE(dev, SND, snd, activate);
+
+	if (activate && test_bit(EV_REP, dev->evbit)) {
+		dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
+		dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
+	}
+}
+
 static int input_start_device(struct input_dev *dev)
 {
 	int error;
@@ -583,6 +625,11 @@ static int input_start_device(struct input_dev *dev)
 			}
 		}
 
+		scoped_guard(spinlock_irq, &dev->event_lock) {
+			dev->ready = true;
+			input_dev_toggle(dev, true);
+		}
+
 		if (dev->poller)
 			input_dev_poller_start(dev->poller);
 	}
@@ -661,6 +708,10 @@ void input_close_device(struct input_handle *handle)
 		if (!--dev->users && !dev->inhibited) {
 			if (dev->poller)
 				input_dev_poller_stop(dev->poller);
+
+			scoped_guard(spinlock_irq, &dev->event_lock)
+				dev->ready = false;
+
 			if (dev->close)
 				dev->close(dev);
 		}
@@ -1712,37 +1763,6 @@ static int input_dev_uevent(const struct device *device, struct kobj_uevent_env
 	return 0;
 }
 
-#define INPUT_DO_TOGGLE(dev, type, bits, on)				\
-	do {								\
-		int i;							\
-		bool active;						\
-									\
-		if (!test_bit(EV_##type, dev->evbit))			\
-			break;						\
-									\
-		for_each_set_bit(i, dev->bits##bit, type##_CNT) {	\
-			active = test_bit(i, dev->bits);		\
-			if (!active && !on)				\
-				continue;				\
-									\
-			dev->event(dev, EV_##type, i, on ? active : 0);	\
-		}							\
-	} while (0)
-
-static void input_dev_toggle(struct input_dev *dev, bool activate)
-{
-	if (!dev->event)
-		return;
-
-	INPUT_DO_TOGGLE(dev, LED, led, activate);
-	INPUT_DO_TOGGLE(dev, SND, snd, activate);
-
-	if (activate && test_bit(EV_REP, dev->evbit)) {
-		dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
-		dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
-	}
-}
-
 /**
  * input_reset_device() - reset/restore the state of input device
  * @dev: input device whose state needs to be reset
@@ -1770,21 +1790,25 @@ static int input_inhibit_device(struct input_dev *dev)
 		return 0;
 
 	if (dev->users) {
-		if (dev->close)
-			dev->close(dev);
 		if (dev->poller)
 			input_dev_poller_stop(dev->poller);
+
+		scoped_guard(spinlock_irq, &dev->event_lock) {
+			input_dev_toggle(dev, false);
+			dev->ready = false;
+		}
+
+		if (dev->close)
+			dev->close(dev);
 	}
 
 	scoped_guard(spinlock_irq, &dev->event_lock) {
 		input_mt_release_slots(dev);
 		input_dev_release_keys(dev);
 		input_handle_event(dev, EV_SYN, SYN_REPORT, 1);
-		input_dev_toggle(dev, false);
+		dev->inhibited = true;
 	}
 
-	dev->inhibited = true;
-
 	return 0;
 }
 
@@ -1804,12 +1828,15 @@ static int input_uninhibit_device(struct input_dev *dev)
 			if (error)
 				return error;
 		}
-	}
 
-	dev->inhibited = false;
+		guard(spinlock_irq)(&dev->event_lock);
+		dev->ready = true;
+	}
 
-	scoped_guard(spinlock_irq, &dev->event_lock)
+	scoped_guard(spinlock_irq, &dev->event_lock) {
+		dev->inhibited = false;
 		input_dev_toggle(dev, true);
+	}
 
 	if (dev->users && dev->poller)
 		input_dev_poller_start(dev->poller);
diff --git a/include/linux/input.h b/include/linux/input.h
index f7a2cfad5448..a307a3f6d5e2 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -128,11 +128,14 @@ enum input_clock_type {
  * @devres_managed: indicates that devices is managed with devres framework
  *	and needs not be explicitly unregistered or freed.
  * @timestamp: storage for a timestamp set by input_set_timestamp called
- *  by a driver
+ *	by a driver
  * @inhibited: indicates that the input device is inhibited. If that is
- * the case then input core ignores any events generated by the device.
- * Device's close() is called when it is being inhibited and its open()
- * is called when it is being uninhibited.
+ *	the case then input core ignores any events generated by the device.
+ *	Device's close() is called when it is being inhibited and its open()
+ *	is called when it is being uninhibited.
+ * @ready: indicates that the device has been successfully opened and is
+ *	prepared to process events (like LEDs or sounds) sent from the
+ *	input core.
  */
 struct input_dev {
 	const char *name;
@@ -209,6 +212,7 @@ struct input_dev {
 	ktime_t timestamp[INPUT_CLK_MAX];
 
 	bool inhibited;
+	bool ready;
 };
 #define to_input_dev(d) container_of(d, struct input_dev, dev)
 
-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH 1/4] Input: fix poller start order on uninhibit

[sashiko-bot]

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

Pre-existing issues:
- [High] Initial events from `dev->open()` are dropped during device uninhibit
--

commit f4fc68b8f579c449b7f9056431e51975c324c92e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: fix poller start order on uninhibit

This commit resolves an issue where initial events generated by the poller
are dropped during a device uninhibit operation. It defers starting the
poller until after the device is fully marked as uninhibited.

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index cf6fecea79b82..d99dfe69e12d8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1793,8 +1793,6 @@ static int input_uninhibit_device(struct input_dev *dev)
>  			if (error)
>  				return error;
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but it appears the exact same
logic flaw exists for dev->open() which is called just above this snippet
in input_uninhibit_device():

    if (dev->open) {
        error = dev->open(dev);
        if (error)
            return error;
    }

Since dev->inhibited is still true when dev->open() is called, if a driver
generates initial synchronous events during its open routine (for example,
gpio_keys_report_state() called by gpio_keys_open()), won't those events
also be silently filtered out by input_get_disposition() returning
INPUT_IGNORE_EVENT?

Since this patch addresses this state mismatch for the poller, should the
dev->open() call also be deferred until after dev->inhibited is set to false?

> -		if (dev->poller)
> -			input_dev_poller_start(dev->poller);
>  	}
>  
>  	dev->inhibited = false;
> @@ -1802,6 +1800,9 @@ static int input_uninhibit_device(struct input_dev *dev)
>  	scoped_guard(spinlock_irq, &dev->event_lock)
>  		input_dev_toggle(dev, true);
>  
> +	if (dev->users && dev->poller)
> +		input_dev_poller_start(dev->poller);
> +
>  	return 0;
>  }

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

Re: [PATCH 4/4] Input: ensure device is ready before delivering events

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing physical feedback teardown in input_close_device causes LEDs/sounds to remain stuck ON indefinitely.
--

commit 75d930ba0fffcfc837221baa51848c81d0ba0c9e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: ensure device is ready before delivering events

This commit introduces a ready flag in the input_dev structure to prevent
events from being delivered to the hardware before the driver's open() method
successfully completes. It ensures logical state is replayed upon opening,
and synchronizes physical feedback teardown during inhibition.

> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 0a95cbdc467ed..724cc146fc093 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c

[ ... ]

> @@ -661,6 +708,10 @@ void input_close_device(struct input_handle *handle)
>  		if (!--dev->users && !dev->inhibited) {
>  			if (dev->poller)
>  				input_dev_poller_stop(dev->poller);
> +
> +			scoped_guard(spinlock_irq, &dev->event_lock)
> +				dev->ready = false;

[Severity: Medium]
Will this leave LEDs and sounds permanently stuck on when a device is closed?

In input_inhibit_device(), there is a call to input_dev_toggle(dev, false)
right before setting dev->ready to false.

However, in input_close_device(), dev->ready is cleared without first
toggling the state off.

Since the suspend and poweroff handlers rely on dev->ready to issue events,
could omitting input_dev_toggle() here prevent them from turning off physical
feedback, resulting in a persistent power drain?

> +
>  			if (dev->close)
>  				dev->close(dev);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624055008.2494980-1-dmitry.torokhov@gmail.com?part=4