[PATCH] media: stm32: dcmi: Switch to __v4l2_subdev_state_alloc()

[Laurent Pinchart]

Hi Marek,

CC'ing Tomi to get his opinion.

On Sun, Jun 19, 2022 at 12:24:42AM +0200, Marek Vasut wrote:
> Any local subdev state should be allocated and free'd using
> __v4l2_subdev_state_alloc()/__v4l2_subdev_state_free(), which
> takes care of calling .init_cfg() subdev op. Without this,
> subdev internal state might be uninitialized by the time
> any other subdev op is called.
> 
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Alain Volmat <alain.volmat at foss.st.com>
> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>
> Cc: Amelie DELAUNAY <amelie.delaunay at foss.st.com>
> Cc: Hugues FRUCHET <hugues.fruchet at foss.st.com>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: Philippe CORNU <philippe.cornu at foss.st.com>
> Cc: linux-stm32 at st-md-mailman.stormreply.com
> Cc: linux-arm-kernel at lists.infradead.org
> ---
>  drivers/media/platform/st/stm32/stm32-dcmi.c | 51 +++++++++++---------
>  1 file changed, 29 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c
> index ec54b5ecfd544..ef795c12fb233 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c
> @@ -996,22 +996,26 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
>  			struct dcmi_framesize *sd_framesize)
>  {
>  	const struct dcmi_format *sd_fmt;
> +	static struct lock_class_key key;
>  	struct dcmi_framesize sd_fsize;
>  	struct v4l2_pix_format *pix = &f->fmt.pix;
> -	struct v4l2_subdev_pad_config pad_cfg;
> -	struct v4l2_subdev_state pad_state = {
> -		.pads = &pad_cfg
> -		};
> +	struct v4l2_subdev_state *sd_state;
>  	struct v4l2_subdev_format format = {
>  		.which = V4L2_SUBDEV_FORMAT_TRY,
>  	};
>  	bool do_crop;
>  	int ret;
>  
> +	sd_state = __v4l2_subdev_state_alloc(dcmi->source, "dcmi:state->lock", &key);
> +	if (IS_ERR(sd_state))
> +		return PTR_ERR(sd_state);
> +
>  	sd_fmt = find_format_by_fourcc(dcmi, pix->pixelformat);
>  	if (!sd_fmt) {
> -		if (!dcmi->num_of_sd_formats)
> -			return -ENODATA;
> +		if (!dcmi->num_of_sd_formats) {
> +			ret = -ENODATA;
> +			goto done;
> +		}
>  
>  		sd_fmt = dcmi->sd_formats[dcmi->num_of_sd_formats - 1];
>  		pix->pixelformat = sd_fmt->fourcc;
> @@ -1036,10 +1040,9 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
>  	}
>  
>  	v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
> -	ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
> -			       &pad_state, &format);
> +	ret = v4l2_subdev_call(dcmi->source, pad, set_fmt, sd_state, &format);
>  	if (ret < 0)
> -		return ret;
> +		goto done;
>  
>  	/* Update pix regarding to what sensor can do */
>  	v4l2_fill_pix_format(pix, &format.format);
> @@ -1079,7 +1082,9 @@ static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
>  	if (sd_framesize)
>  		*sd_framesize = sd_fsize;
>  
> -	return 0;
> +done:
> +	__v4l2_subdev_state_free(sd_state);
> +	return ret;
>  }
>  
>  static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f)
> @@ -1183,31 +1188,33 @@ static int dcmi_set_sensor_format(struct stm32_dcmi *dcmi,
>  				  struct v4l2_pix_format *pix)
>  {
>  	const struct dcmi_format *sd_fmt;
> +	static struct lock_class_key key;
> +	struct v4l2_subdev_state *sd_state;
>  	struct v4l2_subdev_format format = {
>  		.which = V4L2_SUBDEV_FORMAT_TRY,
>  	};
> -	struct v4l2_subdev_pad_config pad_cfg;
> -	struct v4l2_subdev_state pad_state = {
> -		.pads = &pad_cfg
> -		};
>  	int ret;
>  
> +	sd_state = __v4l2_subdev_state_alloc(dcmi->source, "dcmi:state->lock", &key);
> +	if (IS_ERR(sd_state))
> +		return PTR_ERR(sd_state);
> +
>  	sd_fmt = find_format_by_fourcc(dcmi, pix->pixelformat);
>  	if (!sd_fmt) {
> -		if (!dcmi->num_of_sd_formats)
> -			return -ENODATA;
> +		if (!dcmi->num_of_sd_formats) {
> +			ret = -ENODATA;
> +			goto done;
> +		}
>  
>  		sd_fmt = dcmi->sd_formats[dcmi->num_of_sd_formats - 1];
>  		pix->pixelformat = sd_fmt->fourcc;
>  	}
>  
>  	v4l2_fill_mbus_format(&format.format, pix, sd_fmt->mbus_code);
> -	ret = v4l2_subdev_call(dcmi->source, pad, set_fmt,
> -			       &pad_state, &format);
> -	if (ret < 0)
> -		return ret;
> -
> -	return 0;
> +	ret = v4l2_subdev_call(dcmi->source, pad, set_fmt, sd_state, &format);
> +done:
> +	__v4l2_subdev_state_free(sd_state);
> +	return ret;
>  }
>  
>  static int dcmi_get_sensor_bounds(struct stm32_dcmi *dcmi,

-- 
Regards,

Laurent Pinchart