Hi Hans, Thank you for the patch. On Mon, Sep 02, 2024 at 04:04:54PM +0200, Hans Verkuil wrote: > Add two new checks: > > 1) wait_prepare and wait_finish callbacks are either both present or > both unset, you can't mix. > 2) if lock == NULL, then wait_prepare (and due to check 1 also > wait_finish) must be present. > > These checks should prevent the case where lock == NULL, but there > is no way to release/reacquire whatever lock is used when waiting > for a buffer to arrive in VIDIOC_DQBUF. Don't we use the video_device lock when the queue lock is NULL ? Shouldn't it be valid to not set wait_prepare and wait_finish in that case ? > Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> > --- > drivers/media/common/videobuf2/videobuf2-core.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index 29a8d876e6c2..6335ac7b771a 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -2644,6 +2644,14 @@ int vb2_core_queue_init(struct vb2_queue *q) > if (WARN_ON(q->min_reqbufs_allocation > q->max_num_buffers)) > return -EINVAL; > > + /* Either both or none are set */ > + if (WARN_ON(!q->ops->wait_prepare ^ !q->ops->wait_finish)) > + return -EINVAL; > + > + /* Warn if q->lock is NULL and no custom wait_prepare is provided */ > + if (WARN_ON(!q->lock && !q->ops->wait_prepare)) > + return -EINVAL; > + > INIT_LIST_HEAD(&q->queued_list); > INIT_LIST_HEAD(&q->done_list); > spin_lock_init(&q->done_lock); -- Regards, Laurent Pinchart
