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

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 11/26/21 11:08, Paul Kocialkowski wrote:
> 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.

Ack, that makes sense. I will send a v2 with this change.

Regards,

Hans



>> 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
>>
>