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

[Philipp Zabel <p.zabel@pengutronix.de>]

On Fr, 2025-11-07 at 15:44 -0500, Frank Li wrote:
> Use guard() to simplify mutex locking. No functional change.
> 
> Signed-off-by: Frank Li <Frank.Li@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 f8d0b3b8b250774af51f2aba731e639131aceff9..54e9491d3428686288a5bc9bb58a5a0a25aca696 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -413,21 +413,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
> @@ -442,14 +438,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;

This is mixing goto and cleanup in the same function, which the scope-
based cleanup helpers documentation suggests shouldn't be done.

In this case I see no problem because we only jump to update_count from
under the lock, but since issues with gotos around automatic cleanup
are easy to miss, I think it would be better to also turn the
update_count jump into a conditional block.


regards
Philipp