A few more comments:
On 09/11/18 13:58, Johan Fjeldtvedt wrote:
> Warn when driver sets 0 number of planes or 0 as plane sizes.
It should also return an error, since there is no point continuing
with garbage values.
Also add *why* this is useful to the commit. I know why, since I
suggested it to you, but others don't :-)
>
> Signed-off-by: Johan Fjeldtvedt <johfjeld@xxxxxxxxx>
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index f32ec7342ef0..6f903740d813 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> unsigned int num_buffers, allocated_buffers, num_planes = 0;
> unsigned plane_sizes[VB2_MAX_PLANES] = { };
> int ret;
> + int i;
>
> if (q->streaming) {
> dprintk(1, "streaming active\n");
> @@ -718,6 +719,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
> if (ret)
> return ret;
>
> + /* Check that driver has set sane values */
> + WARN_ON(!num_buffers);
Just return an error here. EINVAL is not unreasonable. So:
if (WARN_ON(!num_buffers))
return -EINVAL;
> +
> + for (i = 0; i < num_buffers; i++)
> + WARN_ON(!plane_sizes[i]);
Ditto.
> +
> /* Finally, allocate buffers and video memory */
> allocated_buffers =
> __vb2_queue_alloc(q, memory, num_buffers, num_planes, plane_sizes);
>
Regards,
Hans
