Hi,

On 11/26/21 11:08, Paul Kocialkowski wrote:

Hi Hans,

On Thu 25 Nov 21, 21:31, Hans de Goede wrote:

quoted

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

quoted

Cc: Paul Kocialkowski <redacted>
Signed-off-by: Hans de Goede <redacted>
---
 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