From: Mauro Carvalho Chehab <mchehab@infradead.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Devin Heitmueller <dheitmueller@kernellabs.com>,
Jesper Juhl <jj@chaosbits.net>,
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: Re: [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 11:37:22 -0300 [thread overview]
Message-ID: <4E04A122.2080002@infradead.org> (raw)
In-Reply-To: <201106241554.10751.hverkuil@xs4all.nl>
Em 24-06-2011 10:54, Hans Verkuil escreveu:
> On Friday, June 24, 2011 15:45:59 Devin Heitmueller wrote:
>> On Fri, Jun 24, 2011 at 9:29 AM, Mauro Carvalho Chehab
>> <mchehab@infradead.org> wrote:
>>>> MythTV has a bunch of these too (mainly so the code can adapt to
>>>> driver bugs that are fixed in later revisions). Putting Mauro's patch
>>>> upstream will definitely cause breakage.
>>>
>>> It shouldn't, as ivtv driver version is lower than 3.0.0. All the old bug fixes
>>> aren't needed if version is >= 3.0.0.
>>>
>>> Besides that, trusting on a driver revision number to detect that a bug is
>>> there is not the right thing to do, as version numbers are never increased at
>>> the stable kernels (nor distro modified kernels take care of increasing revision
>>> number as patches are backported there).
>>
>> The versions are increased at the discretion of the driver maintainer,
>> usually when there is some userland visible change in driver behavior.
>> I assure you the application developers don't *want* to rely on such
>> a mechanism, but there have definitely been cases in the past where
>> there was no easy way to detect the behavior of the driver from
>> userland.
>>
>> It lets application developers work around things like violations of
>> the V4L2 standard which get fixed in newer revisions of the driver.
>> It provides them the ability to put a hack in their code that says "if
>> (version < X) then this driver feature is broken and I shouldn't use
>> it."
>
> Indeed. Ideally we shouldn't need it. But reality is different.
>
> What we have right now works and I see no compelling reason to change the
> behavior.
A per-driver version only works if the user is running a vanilla kernel without
any stable patches applied.
I doubt that this covers the large amount of the users: they'll either use an
stable patched kernel or a distribution-specific one. On both cases, the driver
version is not associated with a bug fix, as the driver maintainers just take
care of increasing the driver version once per each new kernel version (when
they care enough).
Also, a git blame for the V4L2 drivers shows that only a few drivers have their
version increased as changes are applied there. So, relying on cap->version
has a minimal chance of working only with a few drivers, with vanilla *.0 kernels.
Anyway, I think that we should at least apply the enclosed patch, and remove
KERNEL_VERSION and linux/version.h includes for the drivers that didn't change
its version in the past 2 kernel releases.
I'll work later on the linux/version.h cleanup patches.
Cheers,
Mauro
-
[media] v4l2-ioctl: Add a default value for kernel version
Most drivers don't increase kernel versions as newer features are added or
bug fixes are solved. So, vidioc_querycap returned value for cap->version is
meaningless. Instead of keeping this situation forever, let's add a default
value matching the current Linux version.
Drivers that want to keep their own version control can still do it, as they
can override the default value for cap->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..61ac6bf 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>
@@ -605,6 +606,7 @@ static long __video_do_ioctl(struct file *file,
if (!ops->vidioc_querycap)
break;
+ cap->version = LINUX_VERSION_CODE;
ret = ops->vidioc_querycap(file, fh, cap);
if (!ret)
dbgarg(cmd, "driver=%s, card=%s, bus=%s, "
next prev parent reply other threads:[~2011-06-24 14:37 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 ` [RFC] Don't use linux/version.h anymore to indicate a per-driver version - Was: " Mauro Carvalho Chehab
2011-06-24 11:26 ` 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 [this message]
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=4E04A122.2080002@infradead.org \
--to=mchehab@infradead.org \
--cc=ceph-devel@vger.kernel.org \
--cc=dheitmueller@kernellabs.com \
--cc=hverkuil@xs4all.nl \
--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