linux-kernel - Re: [PATCH 1/2] media: davinci: vpif capture: upgrade the driver with v4l offerings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5339840D.9000702@xs4all.nl>
Date:	Mon, 31 Mar 2014 17:04:45 +0200
From:	Hans Verkuil <hverkuil@...all.nl>
To:	"Lad, Prabhakar" <prabhakar.csengg@...il.com>,
	LMML <linux-media@...r.kernel.org>,
	DLOS <davinci-linux-open-source@...ux.davincidsp.com>
CC:	LKML <linux-kernel@...r.kernel.org>,
	Hans Verkuil <hans.verkuil@...co.com>,
	Mauro Carvalho Chehab <m.chehab@...sung.com>
Subject: Re: [PATCH 1/2] media: davinci: vpif capture: upgrade the driver
 with v4l offerings

Hi Prabhakar,

This looks really nice!

I'll do a full review on Friday, but in the meantime can you post the output
of 'v4l2-compliance -s' using the latest v4l2-compliance version? I've made
some commits today, so you need to do a git pull of v4l-utils.git.

I also have a small comment below:

On 03/31/2014 04:52 PM, Lad, Prabhakar wrote:
> From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
> 
> This patch upgrades the vpif display driver with
> v4l helpers, this patch does the following,
> 
> 1: initialize the vb2 queue and context at the time of probe
> and removes context at remove() callback.
> 2: uses vb2_ioctl_*() helpers.
> 3: uses vb2_fop_*() helpers.
> 4: uses SIMPLE_DEV_PM_OPS.
> 5: uses vb2_ioctl_*() helpers.
> 6: vidioc_g/s_priority is now handled by v4l core.
> 7: removed driver specific fh and now using one provided by v4l.
> 8: fixes checkpatch warnings.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@...il.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c |  916 ++++++-------------------
>  drivers/media/platform/davinci/vpif_capture.h |   32 +-
>  2 files changed, 229 insertions(+), 719 deletions(-)
> 
> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
> index 8dea0b8..76c15b3 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c

...

>  static int vpif_buffer_init(struct vb2_buffer *vb)
>  {
> -	struct vpif_cap_buffer *buf = container_of(vb,
> -					struct vpif_cap_buffer, vb);
> +	struct vpif_cap_buffer *buf = to_vpif_buffer(vb);
>  
>  	INIT_LIST_HEAD(&buf->list);
>  
>  	return 0;
>  }

Is this really necessary? I think vpif_buffer_init can just be removed.
Ditto for vpif_display.c.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/