Re: [PATCH 3/4] Input: sur40 - factor out and move input device initialization

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Deadlock in `sur40_stop_streaming` due to waiting for buffers that will never be processed if input polling is not active.
--

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

Input: sur40 - factor out and move input device initialization
    
The input device allocation, setup, and registration in sur40_probe() is
quite verbose. Factor it out into a helper function sur40_init_input() to
improve readability.
    
Additionally, call this helper at the very end of sur40_probe() instead of
allocating the input device early. This ensures all video components are fully
initialized before the input device is registered (which starts polling),
and simplifies the early probe error paths since we don't have to carry and
free the input device if probe fails during early V4L2 setup.
    
Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index e9089b0c3e2f4..1ad68131e3a64 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c

[ ... ]

> @@ -647,6 +647,53 @@ static int sur40_input_setup_events(struct input_dev *input_dev)
>  	return 0;
>  }
>  
> +static int sur40_init_input(struct sur40_state *sur40)
> +{
> +	struct input_dev *input;
> +	int error;
> +

[ ... ]

> +	error = sur40_input_setup_events(input);
> +	if (error)
> +		goto err_free_input;
> +
> +	input_set_drvdata(input, sur40);
> +	error = input_setup_polling(input, sur40_poll);

[Severity: High]
This isn't a bug introduced by this patch, but setting up polling here
highlights an existing deadlock in the driver.

If a user opens the video device, queues buffers, starts streaming, and then
stops streaming while the input device remains closed, the sur40_poll()
callback will never run.

This causes a deadlock when stopping the video stream:

drivers/input/touchscreen/sur40.c:sur40_stop_streaming() {
	...
	vb2_wait_for_all_buffers(vq);
	sur40->sequence = -1;

	/* Release all active buffers */
	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
}

The vb2_wait_for_all_buffers() function blocks indefinitely waiting for buffers
to be returned, but those buffers are only processed inside sur40_poll(). Since
the input device is closed, the polling thread isn't active, causing the
process to hang.

Should the driver avoid waiting for buffers if polling isn't active, or
should the polling lifecycle be decoupled from the input device open state?

> +	if (error) {
> +		dev_err(sur40->dev, "failed to set up polling\n");
> +		goto err_free_input;
> +	}

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