linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Junghak Sung <jh1009.sung@samsung.com>,
	linux-media@vger.kernel.org, mchehab@osg.samsung.com,
	laurent.pinchart@ideasonboard.com, sakari.ailus@iki.fi,
	pawel@osciak.com
Cc: inki.dae@samsung.com, sw0312.kim@samsung.com,
	nenggun.kim@samsung.com, sangbae90.lee@samsung.com,
	rany.kwon@samsung.com
Subject: Re: [RFC PATCH v4 6/8] [media] videobuf2: Replace v4l2-specific data with vb2 data.
Date: Fri, 11 Sep 2015 10:14:06 +0200	[thread overview]
Message-ID: <55F28D4E.9@xs4all.nl> (raw)
In-Reply-To: <1441797597-17389-7-git-send-email-jh1009.sung@samsung.com>

Hi Junghak,

A few comments:

On 09/09/2015 01:19 PM, Junghak Sung wrote:
> enum v4l2_memory -> enum vb2_memory
> VIDEO_MAX_FRAME -> VB2_MAX_FRAME
> VIDEO_MAX_PLANES -> VB2_MAX_PLANES

and owner is now a void pointer!

With respect to the two defines above: I think it is a good idea to
add a check to videobuf2-v4l2.c where the compiler compares VIDEO_MAX_FRAME
and VB2_MAX_FRAME (and ditto for MAX_PLANES) and throws an #error if they
do not match.

> 
> Signed-off-by: Junghak Sung <jh1009.sung@samsung.com>
> Signed-off-by: Geunyoung Kim <nenggun.kim@samsung.com>
> Acked-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c |   80 +++++++++++++++---------------
>  include/media/videobuf2-core.h           |   29 +++++++----
>  include/trace/events/v4l2.h              |    5 +-
>  3 files changed, 64 insertions(+), 50 deletions(-)

<snip>

> diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
> index b015b38..b3616ab 100644
> --- a/include/trace/events/v4l2.h
> +++ b/include/trace/events/v4l2.h
> @@ -5,6 +5,7 @@
>  #define _TRACE_V4L2_H
>  
>  #include <linux/tracepoint.h>
> +#include <media/videobuf2-v4l2.h>
>  
>  /* Enums require being exported to userspace, for user tool parsing */
>  #undef EM
> @@ -203,7 +204,9 @@ DECLARE_EVENT_CLASS(vb2_event_class,
>  
>  	TP_fast_assign(
>  		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> -		__entry->minor = q->owner ? q->owner->vdev->minor : -1;
> +		struct v4l2_fh *owner = (struct v4l2_fh *) q->owner;

You don't need a cast here.

> +
> +		__entry->minor = owner ? owner->vdev->minor : -1;
>  		__entry->queued_count = q->queued_count;
>  		__entry->owned_by_drv_count =
>  			atomic_read(&q->owned_by_drv_count);
> 

Regards,

	Hans

  reply	other threads:[~2015-09-11  8:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-09 11:19 [RFC PATCH v4 0/8] Refactoring Videobuf2 for common use Junghak Sung
2015-09-09 11:19 ` [RFC PATCH v4 1/8] [media] videobuf2: Replace videobuf2-core with videobuf2-v4l2 Junghak Sung
2015-09-11  8:02   ` Hans Verkuil
2015-09-14  5:48     ` Junghak Sung
2015-09-14  6:33       ` Hans Verkuil
2015-09-09 11:19 ` [RFC PATCH v4 2/8] [media] videobuf2: Restructure vb2_buffer (1/3) Junghak Sung
2015-09-11  9:19   ` Hans Verkuil
2015-09-09 11:19 ` [RFC PATCH v4 3/8] [media] videobuf2: Restructure vb2_buffer (2/3) Junghak Sung
2015-09-09 11:19 ` [RFC PATCH v4 4/8] [media] videobuf2: Restructure vb2_buffer (3/3) Junghak Sung
2015-09-09 11:19 ` [RFC PATCH v4 5/8] [media] videobuf2: Change queue_setup argument Junghak Sung
2015-09-11  8:52   ` Hans Verkuil
2015-09-11 11:07     ` Junghak Sung
2015-09-11 11:18       ` Hans Verkuil
2015-09-09 11:19 ` [RFC PATCH v4 6/8] [media] videobuf2: Replace v4l2-specific data with vb2 data Junghak Sung
2015-09-11  8:14   ` Hans Verkuil [this message]
2015-09-11 11:12     ` Junghak Sung
2015-09-09 11:19 ` [RFC PATCH v4 7/8] [media] videobuf2: Move v4l2-specific stuff to videobuf2-v4l2 Junghak Sung
2015-09-11  8:17   ` Hans Verkuil
2015-09-09 11:19 ` [RFC PATCH v4 8/8] [media] videobuf2: Remove v4l2-dependencies from videobuf2-core Junghak Sung
2015-09-11 12:47   ` Hans Verkuil
2015-09-14  7:05     ` Junghak Sung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55F28D4E.9@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=inki.dae@samsung.com \
    --cc=jh1009.sung@samsung.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@osg.samsung.com \
    --cc=nenggun.kim@samsung.com \
    --cc=pawel@osciak.com \
    --cc=rany.kwon@samsung.com \
    --cc=sakari.ailus@iki.fi \
    --cc=sangbae90.lee@samsung.com \
    --cc=sw0312.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).