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@xxxxxxxxxxx> >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> 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 >> >
