Hi Sakari, On Thu, Jun 08, 2023 at 12:44:51PM +0000, Sakari Ailus wrote: > On Wed, Jun 07, 2023 at 06:46:49PM +0200, Hans de Goede wrote: > > On ov2680_set_fmt() calls with format->which == V4L2_SUBDEV_FORMAT_TRY, > > ov2680_set_fmt() does not talk to the sensor. > > > > So in this case there is no need to lock the sensor->lock mutex or > > to check that the sensor is streaming. > > > > Fixes: 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver") > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > > --- > > drivers/media/i2c/ov2680.c | 20 +++++++++----------- > > 1 file changed, 9 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > > index d90bbca6e913..a26a6f18f4f1 100644 > > --- a/drivers/media/i2c/ov2680.c > > +++ b/drivers/media/i2c/ov2680.c > > @@ -592,24 +592,22 @@ static int ov2680_set_fmt(struct v4l2_subdev *sd, > > if (format->pad != 0) > > return -EINVAL; > > > > - mutex_lock(&sensor->lock); > > - > > - if (sensor->is_streaming) { > > - ret = -EBUSY; > > - goto unlock; > > - } > > - > > mode = v4l2_find_nearest_size(ov2680_mode_data, > > ARRAY_SIZE(ov2680_mode_data), width, > > height, fmt->width, fmt->height); > > - if (!mode) { > > - ret = -EINVAL; > > - goto unlock; > > - } > > + if (!mode) > > + return -EINVAL; > > > > if (format->which == V4L2_SUBDEV_FORMAT_TRY) { > > try_fmt = v4l2_subdev_get_try_format(sd, sd_state, 0); > > format->format = *try_fmt; > > Access to sd_state needs to be serialised, so mutex should be held here. This operation should be called with the state lock held already, so I don't think any extra locking is needed. > > + return 0; > > + } > > + > > + mutex_lock(&sensor->lock); > > + > > + if (sensor->is_streaming) { > > + ret = -EBUSY; > > goto unlock; > > } > > -- Regards, Laurent Pinchart