Re: [PATCH v2 19/20] media: imx355: Switch to using the subdev state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing state lock acquisition in imx355_set_stream() leading to lockdep BUGs and data races.
--

--- Patch [19]: [PATCH v2 19/20] media: imx355: Switch to using the subdev state ---
commit bbaa67e1db37273f6e97e0631f0e7e19179737fd
Author: Dave Stevenson <dave.stevenson@raspberrypi.com>

media: imx355: Switch to using the subdev state

The commit transitions the imx355 driver to use the v4l2 subdev state for
handling crop and format instead of maintaining its own state variables, and
removes the driver-specific mutex.

> diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> index 10b9cf56e8c56..bb620ebb074f7 100644
> --- a/drivers/media/i2c/imx355.c
> +++ b/drivers/media/i2c/imx355.c
[ ... ]
> @@ -958,12 +893,10 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
>  	struct imx355 *imx355 = to_imx355(sd);
>  	int ret = 0;
>  
> -	mutex_lock(&imx355->mutex);
> -
>  	if (enable) {
>  		ret = pm_runtime_resume_and_get(imx355->dev);
>  		if (ret < 0)
> -			goto err_unlock;
> +			return ret;
>  
>  		/*
>  		 * Apply default & customized values

[Severity: High]
Does this leave imx355_set_stream() entirely lockless?

This function calls imx355_start_streaming(), which calls
v4l2_subdev_get_locked_active_state() to get the subdev state. That function
asserts that the state lock is held.

Without holding the state lock here, won't lockdep unconditionally trigger
BUG splats when calling v4l2_subdev_get_locked_active_state() and later
when calling __v4l2_ctrl_grab() in this function?

Also, without the lock, couldn't these reads race against concurrent
VIDIOC_SUBDEV_S_FMT and VIDIOC_S_CTRL ioctls?

> @@ -981,14 +914,10 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
>  	__v4l2_ctrl_grab(imx355->vflip, enable);
>  	__v4l2_ctrl_grab(imx355->hflip, enable);
>  
> -	mutex_unlock(&imx355->mutex);
> -
>  	return ret;
>  
>  err_rpm_put:
>  	pm_runtime_put_autosuspend(imx355->dev);
> -err_unlock:
> -	mutex_unlock(&imx355->mutex);
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-media-imx355-v2-0-1683ec07b897@raspberrypi.com?part=19