Re: [PATCH RESEND v2 4/5] media: staging: media: imx6-mipi-csi2: use guard() to simplify code

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Frank,

Thank you for the patch.

On Fri, Jan 16, 2026 at 11:17:59AM -0500, Frank Li wrote:
> Use guard() to simplify mutex locking. No functional change.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> leave as it without cleanup goto branch because there are two path to
> update stream_count.
> 
> And it will be replaced soon at
> 
> Use new v4l2_subdev_pad_ops.enable_streams(disalbe_stream) replace
> deprecated s_stream interface.
> 
> https://lore.kernel.org/imx/20250821-95_cam-v3-18-c9286fbb34b9@nxp.com/
> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 54 +++++++++++-------------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index e1b4b7fb53131ce9515b9441d8fc420e85d3e993..762f19ffd0858c952027afa8e0f36fc87246e1ea 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -412,21 +412,17 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	int i, ret = 0;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
> -	if (!csi2->src_sd) {
> -		ret = -EPIPE;
> -		goto out;
> -	}
> +	if (!csi2->src_sd)
> +		return -EPIPE;
>  
>  	for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
>  		if (csi2->sink_linked[i])
>  			break;
>  	}
> -	if (i >= CSI2_NUM_SRC_PADS) {
> -		ret = -EPIPE;
> -		goto out;
> -	}
> +	if (i >= CSI2_NUM_SRC_PADS)
> +		return -EPIPE;
>  
>  	/*
>  	 * enable/disable streaming only if stream_count is
> @@ -441,14 +437,12 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
>  	else
>  		csi2_stop(csi2);
>  	if (ret)
> -		goto out;
> +		return ret;
>  
>  update_count:
>  	csi2->stream_count += enable ? 1 : -1;
>  	if (csi2->stream_count < 0)
>  		csi2->stream_count = 0;
> -out:
> -	mutex_unlock(&csi2->lock);
>  	return ret;
>  }
>  
> @@ -466,32 +460,28 @@ static int csi2_link_setup(struct media_entity *entity,
>  
>  	remote_sd = media_entity_to_v4l2_subdev(remote->entity);
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
>  	if (local->flags & MEDIA_PAD_FL_SOURCE) {
>  		if (flags & MEDIA_LNK_FL_ENABLED) {
> -			if (csi2->sink_linked[local->index - 1]) {
> -				ret = -EBUSY;
> -				goto out;
> -			}
> +			if (csi2->sink_linked[local->index - 1])
> +				return -EBUSY;
> +
>  			csi2->sink_linked[local->index - 1] = true;
>  		} else {
>  			csi2->sink_linked[local->index - 1] = false;
>  		}
>  	} else {
>  		if (flags & MEDIA_LNK_FL_ENABLED) {
> -			if (csi2->src_sd) {
> -				ret = -EBUSY;
> -				goto out;
> -			}
> +			if (csi2->src_sd)
> +				return -EBUSY;
> +
>  			csi2->src_sd = remote_sd;
>  		} else {
>  			csi2->src_sd = NULL;
>  		}
>  	}
>  
> -out:
> -	mutex_unlock(&csi2->lock);
>  	return ret;

You can

	return 0;

here and drop the local ret variable. With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  }
>  
> @@ -512,14 +502,12 @@ static int csi2_get_fmt(struct v4l2_subdev *sd,
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	struct v4l2_mbus_framefmt *fmt;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
>  	fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
>  
>  	sdformat->format = *fmt;
>  
> -	mutex_unlock(&csi2->lock);
> -
>  	return 0;
>  }
>  
> @@ -529,17 +517,14 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
>  {
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	struct v4l2_mbus_framefmt *fmt;
> -	int ret = 0;
>  
>  	if (sdformat->pad >= CSI2_NUM_PADS)
>  		return -EINVAL;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
> -	if (csi2->stream_count > 0) {
> -		ret = -EBUSY;
> -		goto out;
> -	}
> +	if (csi2->stream_count > 0)
> +		return -EBUSY;
>  
>  	/* Output pads mirror active input pad, no limits on input pads */
>  	if (sdformat->pad != CSI2_SINK_PAD)
> @@ -548,9 +533,8 @@ static int csi2_set_fmt(struct v4l2_subdev *sd,
>  	fmt = __csi2_get_fmt(csi2, sd_state, sdformat->pad, sdformat->which);
>  
>  	*fmt = sdformat->format;
> -out:
> -	mutex_unlock(&csi2->lock);
> -	return ret;
> +
> +	return 0;
>  }
>  
>  static int csi2_registered(struct v4l2_subdev *sd)

-- 
Regards,

Laurent Pinchart