Re: [PATCH] media: i2c: ov5648: Fix lockdep error

[Paul Kocialkowski <paul.kocialkowski@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

Hi Hans,

On Thu 25 Nov 21, 21:31, Hans de Goede wrote:
> ov5648_state_init() calls ov5648_state_mipi_configure() which uses
> __v4l2_ctrl_s_ctrl[_int64](). This means that sensor->mutex (which
> is also sensor->ctrls.handler.lock) must be locked before calling
> ov5648_state_init().
> 
> ov5648_state_mipi_configure() is also used in other places where
> the lock is already held so it cannot be changed itself.
> 
> Note this is based on an identical (tested) fix for the ov8865 driver,
> this has only been compile-tested.

Good catch, thanks!

Though I think it would look slightly better to have the lock in the
ov5648_state_init function itself rather than in the probe.
I guess the same applies to the ov8865 fix, if it's not too late.

Cheers,

Paul

> Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/media/i2c/ov5648.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov5648.c b/drivers/media/i2c/ov5648.c
> index 947d437ed0ef..9f24bdccf50f 100644
> --- a/drivers/media/i2c/ov5648.c
> +++ b/drivers/media/i2c/ov5648.c
> @@ -2546,7 +2546,9 @@ static int ov5648_probe(struct i2c_client *client)
>  	if (ret)
>  		goto error_mutex;
>  
> +	mutex_lock(&sensor->mutex);
>  	ret = ov5648_state_init(sensor);
> +	mutex_unlock(&sensor->mutex);
>  	if (ret)
>  		goto error_ctrls;
>  
> -- 
> 2.33.1
> 

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]