Re: [PATCH 3/5] media: imx: imx-mipi-csis: Remove lock from s_stream

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

Hi Jacopo,

Thank you for the patch.

On Mon, Mar 14, 2022 at 11:39:39AM +0100, Jacopo Mondi wrote:
> The s_stream() operation implementation was locked to avoid races with
> the mipi_csis_log_status() which access the platform registers and needs
> the interface to be powered up.
> 
> As powering is now handled by runtime_pm which is refcounted, it is
> not necessary to manually lock s_stream() anymore.

The lock is also used to protect against races between .s_stream() and
.set_fmt() both accessing the active format. I don't think we can drop
it.

> As the error path is now simplified, we can inline it.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/platform/imx/imx-mipi-csis.c | 23 ++++++----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/imx/imx-mipi-csis.c b/drivers/media/platform/imx/imx-mipi-csis.c
> index b76bb129a416..4a6152c13d52 100644
> --- a/drivers/media/platform/imx/imx-mipi-csis.c
> +++ b/drivers/media/platform/imx/imx-mipi-csis.c
> @@ -912,16 +912,12 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
>  	int ret;
>  
>  	if (!enable) {
> -		mutex_lock(&csis->lock);
> -
>  		v4l2_subdev_call(csis->src_sd, video, s_stream, 0);
>  
>  		mipi_csis_stop_stream(csis);
>  		if (csis->debug.enable)
>  			mipi_csis_log_counters(csis, true);
>  
> -		mutex_unlock(&csis->lock);
> -
>  		pm_runtime_put(csis->dev);
>  
>  		return 0;
> @@ -937,25 +933,18 @@ static int mipi_csis_s_stream(struct v4l2_subdev *sd, int enable)
>  	if (ret < 0)
>  		return ret;
>  
> -	mutex_lock(&csis->lock);
> -
>  	mipi_csis_start_stream(csis);
>  	ret = v4l2_subdev_call(csis->src_sd, video, s_stream, 1);
> -	if (ret < 0)
> -		goto out;
> +	if (ret < 0) {
> +		mipi_csis_stop_stream(csis);
> +		pm_runtime_put(csis->dev);
>  
> -	mipi_csis_log_counters(csis, true);
> +		return ret;
> +	}
>  
> -	mutex_unlock(&csis->lock);
> +	mipi_csis_log_counters(csis, true);
>  
>  	return 0;
> -
> -out:
> -	mipi_csis_stop_stream(csis);
> -	mutex_unlock(&csis->lock);
> -	pm_runtime_put(csis->dev);
> -
> -	return ret;
>  }
>  
>  static struct v4l2_mbus_framefmt *

-- 
Regards,

Laurent Pinchart