Hi Johan,
Thank you for the patch, but I have some small comments:
On 09/11/18 13:28, Johan Fjeldtvedt wrote:
> Warn when driver sets 0 number of planes or 0 as plane sizes.
> ---
> 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 f32ec7342ef0..d3bc94477e6b 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,13 @@ 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 == 0);
> +
> + for (i = 0; i < num_buffers; i++) {
Linux coding style is not to use curly brackets around single statements.
> + WARN_ON(plane_sizes[i] == 0);
There is a general preference in the kernel to use !foo instead of 'foo == 0'
You can (and should) run the patch against checkpatch:
git diff | scripts/checkpatch.pl --strict
> + }
> +
> /* Finally, allocate buffers and video memory */
> allocated_buffers =
> __vb2_queue_alloc(q, memory, num_buffers, num_planes, plane_sizes);
>
Regards,
Hans
