Re: [PATCH v3 8/8] media: v4l: subdev: Also assert acquired mutex for non-stream drivers

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

Hi Sakari,

Thank you for the patch.

On Mon, Oct 23, 2023 at 08:44:08PM +0300, Sakari Ailus wrote:
> Assert mutex is taken by drivers that are not stream-aware.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Do you expect the assertions to trigger ?

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

> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index e35226587244..8d079ad3287e 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -1680,6 +1680,8 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state,
>  	if (WARN_ON(!state))
>  		return NULL;
>  
> +	lockdep_assert_held(state->lock);
> +
>  	if (state->pads) {
>  		if (stream)
>  			return NULL;
> @@ -1690,8 +1692,6 @@ __v4l2_subdev_state_get_format(struct v4l2_subdev_state *state,
>  		return &state->pads[pad].try_fmt;
>  	}
>  
> -	lockdep_assert_held(state->lock);
> -
>  	stream_configs = &state->stream_configs;
>  
>  	for (i = 0; i < stream_configs->num_configs; ++i) {
> @@ -1714,6 +1714,8 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
>  	if (WARN_ON(!state))
>  		return NULL;
>  
> +	lockdep_assert_held(state->lock);
> +
>  	if (state->pads) {
>  		if (stream)
>  			return NULL;
> @@ -1724,8 +1726,6 @@ __v4l2_subdev_state_get_crop(struct v4l2_subdev_state *state, unsigned int pad,
>  		return &state->pads[pad].try_crop;
>  	}
>  
> -	lockdep_assert_held(state->lock);
> -
>  	stream_configs = &state->stream_configs;
>  
>  	for (i = 0; i < stream_configs->num_configs; ++i) {
> @@ -1748,6 +1748,8 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
>  	if (WARN_ON(!state))
>  		return NULL;
>  
> +	lockdep_assert_held(state->lock);
> +
>  	if (state->pads) {
>  		if (stream)
>  			return NULL;
> @@ -1758,8 +1760,6 @@ __v4l2_subdev_state_get_compose(struct v4l2_subdev_state *state,
>  		return &state->pads[pad].try_compose;
>  	}
>  
> -	lockdep_assert_held(state->lock);
> -
>  	stream_configs = &state->stream_configs;
>  
>  	for (i = 0; i < stream_configs->num_configs; ++i) {

-- 
Regards,

Laurent Pinchart