public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@infradead.org>
To: Jesper Juhl <jj@chaosbits.net>
Cc: LKML <linux-kernel@vger.kernel.org>,
	trivial@kernel.org, linux-media@vger.kernel.org,
	ceph-devel@vger.kernel.org, Sage Weil <sage@newdream.net>
Subject: [RFC] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/
Date: Fri, 24 Jun 2011 08:21:14 -0300	[thread overview]
Message-ID: <4E04732A.3060305@infradead.org> (raw)
In-Reply-To: <alpine.LNX.2.00.1106232356530.17688@swampdragon.chaosbits.net>

Em 23-06-2011 18:58, Jesper Juhl escreveu:
> It was pointed out by 'make versioncheck' that some includes of
> linux/version.h were not needed in include/.
> This patch removes them.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>
> ---
>  include/linux/ceph/messenger.h |    1 -
>  include/media/pwc-ioctl.h      |    1 -
>  2 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
> index 31d91a6..291aa6e 100644
> --- a/include/linux/ceph/messenger.h
> +++ b/include/linux/ceph/messenger.h
> @@ -6,7 +6,6 @@
>  #include <linux/net.h>
>  #include <linux/radix-tree.h>
>  #include <linux/uio.h>
> -#include <linux/version.h>
>  #include <linux/workqueue.h>
>  
>  #include "types.h"
> diff --git a/include/media/pwc-ioctl.h b/include/media/pwc-ioctl.h
> index 0f19779..1ed1e61 100644
> --- a/include/media/pwc-ioctl.h
> +++ b/include/media/pwc-ioctl.h
> @@ -53,7 +53,6 @@
>   */
>  
>  #include <linux/types.h>
> -#include <linux/version.h>
>  
>  /* Enumeration of image sizes */
>  #define PSZ_SQCIF	0x00


The usage of version.h at the Linux media kernel is due to a V4L2 API requirement[1],
where an ioctl query of VIDIOC_QUERYCAP type would return the driver version formatted
with KERNEL_VERSION() macro.

[1] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-querycap.html

While a few driver maintainers are careful enough to increment it on every new
kernel version where the driver was touched, others simply keep it outdated.

IMHO, it doesn't make much sense on having a per-driver version field: the V4L2 layer
should be enough to abstract hardware differences, and to avoid userspace to have a per
driver list of hacks. I don't think that the userspace applications are really using it.
Module versions should just use the MODULE_VERSION() macro.

So, IMO, the better would be to convert this field into a V4L2 API version field
instead like the enclosed patch. Of course, this also means to change the V4L2 API
Docbook. After that, we can cleanup all those linux/version.h code on all V4L drivers.

The idea is that, every time we add something new at the V4L2 API, we'll increment it
to match the current kernel version.

On a quick look, all drivers, except by one uses versions <= KERNEL_VERSION(3, 0, 0).
The only exception is the pwc driver, with version is KERNEL_VERSION(10, 0, 12). Due to
a bug on it, it also reports its version as: "10.0.14" at module version. The version
10.0.12 is reported there since 2006, even having suffered a major change, due to the
removal of the V4L1 API, on changeset 479567ce3af7b99d645a3c53b8ca2fc65e46efdc.
So, I think it would be safe to change it to 3.0.0, as using the version here, in the favor
of a greater good. We can keep the driver-specific version only at 

Comments?

If others are ok with that, I'll prepare the changesets.

Cheers,
Mauro


-

[media] v4l2 core: Use a per-API version instead of a per driver version

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 213ba7d..d8fa571 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/kernel.h>diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 213ba7d..b19ad56 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
+#include <linux/version.h>
 
 #include <linux/videodev2.h>
 
@@ -27,6 +28,8 @@
 #include <media/v4l2-device.h>
 #include <media/v4l2-chip-ident.h>
 
+#define V4L2_API_VERSION KERNEL_VERSION(3, 0, 0)
+
 #define dbgarg(cmd, fmt, arg...) \
 		do {							\
 		    if (vfd->debug & V4L2_DEBUG_IOCTL_ARG) {		\
@@ -606,13 +609,16 @@ static long __video_do_ioctl(struct file *file,
 			break;
 
 		ret = ops->vidioc_querycap(file, fh, cap);
-		if (!ret)
+		if (!ret) {
+			cap->version = V4L2_API_VERSION;
+
 			dbgarg(cmd, "driver=%s, card=%s, bus=%s, "
 					"version=0x%08x, "
 					"capabilities=0x%08x\n",
 					cap->driver, cap->card, cap->bus_info,
 					cap->version,
 					cap->capabilities);
+		}
 		break;
 	}
 

  parent reply	other threads:[~2011-06-24 11:21 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <alpine.LNX.2.00.1106232344480.17688@swampdragon.chaosbits.net>
2011-06-23 21:58 ` [PATCH 03/37] Remove unneeded version.h includes from include/ Jesper Juhl
2011-06-23 22:15   ` Sage Weil
2011-06-24 11:21   ` Mauro Carvalho Chehab [this message]
2011-06-24 11:26     ` [RFC] Don't use linux/version.h anymore to indicate a per-driver version - Was: " Hans Verkuil
2011-06-24 12:20       ` Devin Heitmueller
2011-06-24 13:29         ` Mauro Carvalho Chehab
2011-06-24 13:45           ` Devin Heitmueller
2011-06-24 13:54             ` Hans Verkuil
2011-06-24 14:37               ` Mauro Carvalho Chehab
2011-06-24 18:25                 ` [PATCH] [media] Stop using linux/version.h on most drivers Mauro Carvalho Chehab
2011-06-25 10:09                   ` Hans Verkuil
2011-06-25 12:14                     ` Mauro Carvalho Chehab
2011-06-27  0:59                       ` Laurent Pinchart
2011-06-24 18:34                 ` [RFC] Don't use linux/version.h anymore to indicate a per-driver version - Was: Re: [PATCH 03/37] Remove unneeded version.h includes from include/ Stefan Richter
2011-06-24 18:48                   ` Devin Heitmueller
2011-06-24 21:04                     ` Andy Walls
2011-06-24 21:20                       ` Stefan Richter
2011-06-24 21:22                         ` Devin Heitmueller
2011-06-24 21:49                           ` Mauro Carvalho Chehab
2011-06-24 22:39                           ` Stefan Richter
2011-06-24 23:02                             ` Mauro Carvalho Chehab
2011-06-24 22:16                       ` Mauro Carvalho Chehab
2011-06-24 22:57                         ` Andy Walls
2011-06-24 21:10                     ` Stefan Richter
2011-06-24 21:52                       ` Mauro Carvalho Chehab
2011-06-24 21:44                   ` Mauro Carvalho Chehab
2011-06-29 21:54         ` Mauro Carvalho Chehab
2011-06-23 22:11 ` [PATCH 10/37] Remove unneeded version.h includes from drivers/media/dvb/ Jesper Juhl
2011-06-23 22:14 ` [PATCH 11/37] Remove unneeded version.h includes (and add where needed) for drivers/media/radio/ Jesper Juhl
2011-06-23 22:17 ` [PATCH 12/37] Remove unneeded version.h includes (and add where needed) for drivers/media/video/ Jesper Juhl
2011-06-24  8:52   ` Laurent Pinchart

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=4E04732A.3060305@infradead.org \
    --to=mchehab@infradead.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=jj@chaosbits.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=sage@newdream.net \
    --cc=trivial@kernel.org \
    /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