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@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 > -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature