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/
Em 24-06-2011 09:20, Devin Heitmueller escreveu: Also, it screws up the ability for users to get fixes through the media_build tree (unless you are increasing the revision constantly with every merge you do). Patches merged, and media_build modified in order to use the V4L2 stack version, instead of the kernel one. So, while I'm using a 2.6.32 kernel: $ uname -a Linux pedra 2.6.32-131.0.15.el6.x86_64 #1 SMP Tue May 10 15:42:40 EDT 2011 x86_64 x86_64 x86_64 GNU/Linux Driver reports version 3.0.0: $ v4l2-ctl -D Driver Info (not using libv4l2): Driver name : vivi Card type : vivi Bus info : vivi-000 Driver version: 3.0.0 Capabilities : 0x0501 Video Capture Read/Write Streaming It may be a good idea to increment the extraver number to be, for example, 3.0.99 (or to just decrement 1 number), in order to reflect that this driver is not the vanilla 3.0.0, but, instead, a backported one, otherwise, it will report the version from the last git backport we merge at media_tree.git. If we just subtract 1, we'll have (right now): $ v4l2-ctl -D Driver Info (not using libv4l2): Driver name : vivi Card type : vivi Bus info : vivi-000 Driver version: 2.255.255 Capabilities : 0x0501 Video Capture Read/Write Streaming Which looks somewhat weird, but, after the 3.1 merge window, it will be 3.0.255, with would be nice. So, if we go this way, the better is to wait until 3.1-rc1 before applying it. Comments? PS.: The media_build is not optimized in the sense that a version increment will make it recompile everything. I'll likely fix it if it bothers me enough ;) Thanks, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Friday, June 24, 2011 13:21:14 Mauro Carvalho Chehab wrote:
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 j...@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.
Applications are certainly using it. I know this for a fact for the ivtv driver
where
feature improvements are marked that way.
Without more research on how this is used I am not comfortable with this.
Regards,
Hans
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 mche...@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.hdiff --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);
+ }
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/
Applications are certainly using it. I know this for a fact for the ivtv driver where feature improvements are marked that way. Without more research on how this is used I am not comfortable with this. Regards, Hans 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. Also, it screws up the ability for users to get fixes through the media_build tree (unless you are increasing the revision constantly with every merge you do). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Fri, Jun 24, 2011 at 9:29 AM, Mauro Carvalho Chehab mche...@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. In other words, relying on it doesn't work fine. It's the best (and really only solution) we have today. Also, it screws up the ability for users to get fixes through the media_build tree (unless you are increasing the revision constantly with every merge you do). Why? Developers don't increase version numbers on every applied patch (with is great, as it avoids merge conflicts). The driver maintainer doesn't *have* to increase the version - he does it when he thinks it's appropriate. The point is you are taking that discretion out of *their* hands, and you yourself are unaware of when it is actually needed. You need to stop looking at this from a purist standpoint and think of how application developers actually use the API. They need tools like this to allow them to work around driver bugs while having a source codebase which operates against different kernels (including kernels that may still have those bugs). Sure, in a perfect world where drivers don't have bugs and applications don't have to run against older kernels, what you are saying is not illogical. But then again, we don't live in a perfect world. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Friday, June 24, 2011 15:45:59 Devin Heitmueller wrote: On Fri, Jun 24, 2011 at 9:29 AM, Mauro Carvalho Chehab mche...@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. Regards, Hans In other words, relying on it doesn't work fine. It's the best (and really only solution) we have today. Also, it screws up the ability for users to get fixes through the media_build tree (unless you are increasing the revision constantly with every merge you do). Why? Developers don't increase version numbers on every applied patch (with is great, as it avoids merge conflicts). The driver maintainer doesn't *have* to increase the version - he does it when he thinks it's appropriate. The point is you are taking that discretion out of *their* hands, and you yourself are unaware of when it is actually needed. You need to stop looking at this from a purist standpoint and think of how application developers actually use the API. They need tools like this to allow them to work around driver bugs while having a source codebase which operates against different kernels (including kernels that may still have those bugs). Sure, in a perfect world where drivers don't have bugs and applications don't have to run against older kernels, what you are saying is not illogical. But then again, we don't live in a perfect world. Devin -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
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 mche...@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 mche...@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, -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Jun 24 Mauro Carvalho Chehab wrote: Em 24-06-2011 10:54, Hans Verkuil escreveu: On Friday, June 24, 2011 15:45:59 Devin Heitmueller wrote: 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. If the driver version is in fact an ABI version, then the driver author should really increase it only when ABI behavior is changed (and only if the behavior change can only be communicated by version number --- e.g. addition of an ioctl is not among such reasons). And the author should commit behavior changing implementation and version number change in a single changeset. And anybody who backmerges such an ABI behavior change into another kernel branch (stable, longterm, distro...) must backmerge the associated version number change too. Of course sometimes people realize this only after the fact. Or driver authors don't have a clear understanding of ABI versioning to begin with. I am saying so because I had to learn it too; I certainly wasn't born with an instinct knowledge how to do it properly. (Disclaimer: I have no stake in drivers/media/ ABIs. But I am involved in maintaining a userspace ABI elsewhere in drivers/firewire/, and one of the userspace libraries that use this ABI.) -- Stefan Richter -=-==-== -==- ==--- http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Fri, Jun 24, 2011 at 2:34 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: If the driver version is in fact an ABI version, then the driver author should really increase it only when ABI behavior is changed (and only if the behavior change can only be communicated by version number --- e.g. addition of an ioctl is not among such reasons). And the author should commit behavior changing implementation and version number change in a single changeset. And anybody who backmerges such an ABI behavior change into another kernel branch (stable, longterm, distro...) must backmerge the associated version number change too. Of course sometimes people realize this only after the fact. Or driver authors don't have a clear understanding of ABI versioning to begin with. I am saying so because I had to learn it too; I certainly wasn't born with an instinct knowledge how to do it properly. (Disclaimer: I have no stake in drivers/media/ ABIs. But I am involved in maintaining a userspace ABI elsewhere in drivers/firewire/, and one of the userspace libraries that use this ABI.) Hi Stefan, To be clear, I don't think anyone is actually proposing that the driver version number really be used as any form of formal ABI versioning scheme. In almost all cases, it's so the application can know to *not* do something is the driver is older than X. Given all the cases I've seen, it doesn't really hurt anything if the driver contains a fix from newer than X, aside from the fact that the application won't take advantage of whatever feature/functionality the fix made work. In other words, I think from a backport standpoint, it usually doesn't *hurt* anything if a fix is backported without the version being incremented, aside from applications not knowing that the feature/fix is present. Really, this is all about applications being able to jam a hack into their code that translates to don't call this ioctl() with some particular argument if it's driver W less than version X, because the driver had a bug that is likely to panic the guy's PC. Sure, it's a crummy solution, but at this point it's the best that we have got. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Fri, 2011-06-24 at 14:48 -0400, Devin Heitmueller wrote: On Fri, Jun 24, 2011 at 2:34 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: If the driver version is in fact an ABI version, then the driver author should really increase it only when ABI behavior is changed (and only if the behavior change can only be communicated by version number --- e.g. addition of an ioctl is not among such reasons). And the author should commit behavior changing implementation and version number change in a single changeset. And anybody who backmerges such an ABI behavior change into another kernel branch (stable, longterm, distro...) must backmerge the associated version number change too. Of course sometimes people realize this only after the fact. Or driver authors don't have a clear understanding of ABI versioning to begin with. I am saying so because I had to learn it too; I certainly wasn't born with an instinct knowledge how to do it properly. (Disclaimer: I have no stake in drivers/media/ ABIs. But I am involved in maintaining a userspace ABI elsewhere in drivers/firewire/, and one of the userspace libraries that use this ABI.) Hi Stefan, To be clear, I don't think anyone is actually proposing that the driver version number really be used as any form of formal ABI versioning scheme. In almost all cases, it's so the application can know to *not* do something is the driver is older than X. MythTV, for example, used to use the driver version to work around old VBI bugs and MPEG encoder quirks that the older version of the driver may not have known how to handle: https://github.com/MythTV/mythtv/blob/b98d3a98e3187000ae652df5ffebe2beb5221ba7/mythtv/libs/libmythtv/mpegrecorder.cpp#L335 But for newer versions, MythTV could avoid using its own odd hacks. The bleeding edge MythTV now has most of these removed. Given all the cases I've seen, it doesn't really hurt anything if the driver contains a fix from newer than X, aside from the fact that the application won't take advantage of whatever feature/functionality the fix made work. In other words, I think from a backport standpoint, it usually doesn't *hurt* anything if a fix is backported without the version being incremented, aside from applications not knowing that the feature/fix is present. That seems to be the case to me. Really, this is all about applications being able to jam a hack into their code that translates to don't call this ioctl() with some particular argument if it's driver W less than version X, because the driver had a bug that is likely to panic the guy's PC. Well, not even panics per se, but some thing like the VBI is broken, or the volume control doesn't work, IR blaster is works for this version, or something else stupid that is very visible to the end user. I also use the driver version for troubleshooting problem with users. I roughly know what wasn't working in what version of the cx18 and ivtv drivers. If the end user can tell me the driver version (using v4l2-ctl --log-status) along with his symptoms, it makes my life easier. Being able to efficiently help the end user is a win for both me and the end user. Sure, it's a crummy solution, but at this point it's the best that we have got Yup. We do have crummier solutions: Telling the end user to read their kernel source code to figure out what bugs their driver release has, and to then adjust their application command line arguments accordingly. ;) Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Jun 24 Devin Heitmueller wrote: Really, this is all about applications being able to jam a hack into their code that translates to don't call this ioctl() with some particular argument if it's driver W less than version X, because the driver had a bug that is likely to panic the guy's PC. Sure, it's a crummy solution, but at this point it's the best that we have got. The second best. The best that we have got is that the user runs a fixed kernel. Anyway; if this is the only purpose that this interface version¹ serves, then Mauro's subsystem-centralized solution has the benefit that it eliminates mistakes due to oversight by individual driver authors. Especially because the kind of implementation behavior changes that are tracked by this type of version datum are sometimes just discovered or documented in hindsight. On the other hand, Mauro's solution is redundant to the uname(2) syscall. ¹) Yes, it is still an ABI version, nothing less. With all its backwards and forwards compatibility ramifications. -- Stefan Richter -=-==-== -==- ==--- http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Jun 24 Andy Walls wrote: I also use the driver version for troubleshooting problem with users. I roughly know what wasn't working in what version of the cx18 and ivtv drivers. If the end user can tell me the driver version (using v4l2-ctl --log-status) along with his symptoms, it makes my life easier. Easier: I run Ubuntu 10.4. I run kernel 2.6.32. One of these is usually already included in the first post or IRC message from the user. Separate driver versions are only needed on platforms where drivers are not distributed by the operating system distributor, or driver source code is not released within kernel source code. -- Stefan Richter -=-==-== -==- ==--- http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Fri, Jun 24, 2011 at 5:20 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: Easier: I run Ubuntu 10.4. I run kernel 2.6.32. One of these is usually already included in the first post or IRC message from the user. Separate driver versions are only needed on platforms where drivers are not distributed by the operating system distributor, or driver source code is not released within kernel source code. Unfortunately, this doesn't work as all too often the user has Ubuntu 10.1 but I installed the latest media_build tree a few months ago. Hence they are not necessarily on a particular binary release from a distro but rather have a mix of a distro's binary release and a v4l-dvb tree compiled from source. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
Em 24-06-2011 15:34, Stefan Richter escreveu: On Jun 24 Mauro Carvalho Chehab wrote: Em 24-06-2011 10:54, Hans Verkuil escreveu: On Friday, June 24, 2011 15:45:59 Devin Heitmueller wrote: 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. If the driver version is in fact an ABI version, then the driver author should really increase it only when ABI behavior is changed (and only if the behavior change can only be communicated by version number --- e.g. addition of an ioctl is not among such reasons). And the author should commit behavior changing implementation and version number change in a single changeset. Yes, but driver version were never used as such. Several drivers got lots of updates, ABI change behavior (like the removal of V4L1 API), etc without having a single driver version increment. Other drivers increase it even on minor changes. IMO, it makes no sense on keeping it, but removing this field would break userspace, as a few programs seem to use it. And anybody who backmerges such an ABI behavior change into another kernel branch (stable, longterm, distro...) must backmerge the associated version number change too. Yes, but, again, this doesn't happen. In general, the drivers that use it either increment the version number on a separate patch, or integrate it with one of the patches. It is easy to take a look at ivtv, as it has a separate file with the version number: $ git log --oneline drivers/media/video/ivtv/ivtv-version.h 4359e5b V4L/DVB: ivtv: Increment driver version due to firmware loading changes c019f90 V4L/DVB (10965): ivtv: bump version c58dc0b V4L/DVB (8633): ivtv: update ivtv version number be303e1 V4L/DVB (7930): ivtv: bump version to 1.3.0. fcbbf6f V4L/DVB (7759): ivtv: increase version number to 1.2.1 0170a48 V4L/DVB (6762): ivtv: update version number to 1.2 612570f V4L/DVB (6091): ivtv: header cleanup f38a798 V4L/DVB (5909): ivtv: update version to 1.1 to mark ivtv-fb support 1a0adaf V4L/DVB (5345): ivtv driver for Conexant cx23416/cx23415 MPEG encoder/decoder Looking at the details of the above commits, on several cases there's no explanation why the version was incremented, or why an userspace application should bother to have any special treatment for that version or for the previous one. The date of the commits also don't help much: Date: Sat Jun 12 13:55:33 2010 -0300 Date: Wed Mar 11 18:50:04 2009 -0300 Date: Wed Sep 3 16:46:58 2008 -0300 Date: Sat May 24 12:43:43 2008 -0300 Date: Sat Apr 26 09:43:22 2008 -0300 Date: Fri Dec 7 20:31:17 2007 -0300 Date: Thu Aug 23 05:42:59 2007 -0300 Date: Sun Jul 22 08:39:43 2007 -0300 Date: Fri Apr 27 12:31:25 2007 -0300 Even when there seems to have a good reason for version bump, like on this example (from cx18 driver): commit 9982be8 Author: Andy Walls awa...@radix.net Date: Wed Apr 15 20:49:19 2009 -0300 V4L/DVB (11620): cx18: Increment version due to significant buffer handling changes Version bump from 1.1.0 to 1.2.0 Signed-off-by: Andy Walls awa...@radix.net Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com The commit message doesn't help to tell the application developer how version 1.1.0 is different than version 1.2.0. Also, as this is on a separate commit, if the buffer changes were backported into a stable or distro kernel, the application will have no way to detect the differences. On several cases, the version upgrade is simply due to the addition of a new type of support, like this one: commit 437b775
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/
Em 24-06-2011 18:22, Devin Heitmueller escreveu: On Fri, Jun 24, 2011 at 5:20 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: Easier: I run Ubuntu 10.4. I run kernel 2.6.32. One of these is usually already included in the first post or IRC message from the user. Separate driver versions are only needed on platforms where drivers are not distributed by the operating system distributor, or driver source code is not released within kernel source code. Unfortunately, this doesn't work as all too often the user has Ubuntu 10.1 but I installed the latest media_build tree a few months ago. Hence they are not necessarily on a particular binary release from a distro but rather have a mix of a distro's binary release and a v4l-dvb tree compiled from source. # modprobe vivi # dmesg WARNING: You are using an experimental version of the media stack. As the driver is backported to an older kernel, it doesn't offer enough quality for its usage in production. Use it with care. Latest git patches (needed if you report a bug to linux-media@vger.kernel.org): d3302689d697a99d565ea37c8fb9a19a1a5d0f06 [media] rc-core: fix winbond-cir issues 6337ae50f81c99efbf9fa924c9d1b8b51efbcef2 [media] rc/redrat3: dereferencing null pointer ad0fc4c9ac8bf871b7bf874b2cd34b6b65f099c7 [media] rc: double unlock in rc_register_device() vivi-000: V4L2 device registered as video0 Video Technology Magazine Virtual Video Capture Board ver 0.8.0 successfully loaded. The git changeset is a way better than a version number. Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
Em 24-06-2011 18:10, Stefan Richter escreveu: On Jun 24 Devin Heitmueller wrote: Really, this is all about applications being able to jam a hack into their code that translates to don't call this ioctl() with some particular argument if it's driver W less than version X, because the driver had a bug that is likely to panic the guy's PC. Sure, it's a crummy solution, but at this point it's the best that we have got. The second best. The best that we have got is that the user runs a fixed kernel. Anyway; if this is the only purpose that this interface version¹ serves, then Mauro's subsystem-centralized solution has the benefit that it eliminates mistakes due to oversight by individual driver authors. Especially because the kind of implementation behavior changes that are tracked by this type of version datum are sometimes just discovered or documented in hindsight. On the other hand, Mauro's solution is redundant to the uname(2) syscall. Yes. That's why my initial proposal were to add some value to it by not associating it with the kernel version, but with a number that will be incremented only if the V4L2 API changes. ¹) Yes, it is still an ABI version, nothing less. With all its backwards and forwards compatibility ramifications. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
Em 24-06-2011 18:04, Andy Walls escreveu: On Fri, 2011-06-24 at 14:48 -0400, Devin Heitmueller wrote: On Fri, Jun 24, 2011 at 2:34 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: If the driver version is in fact an ABI version, then the driver author should really increase it only when ABI behavior is changed (and only if the behavior change can only be communicated by version number --- e.g. addition of an ioctl is not among such reasons). And the author should commit behavior changing implementation and version number change in a single changeset. And anybody who backmerges such an ABI behavior change into another kernel branch (stable, longterm, distro...) must backmerge the associated version number change too. Of course sometimes people realize this only after the fact. Or driver authors don't have a clear understanding of ABI versioning to begin with. I am saying so because I had to learn it too; I certainly wasn't born with an instinct knowledge how to do it properly. (Disclaimer: I have no stake in drivers/media/ ABIs. But I am involved in maintaining a userspace ABI elsewhere in drivers/firewire/, and one of the userspace libraries that use this ABI.) Hi Stefan, To be clear, I don't think anyone is actually proposing that the driver version number really be used as any form of formal ABI versioning scheme. In almost all cases, it's so the application can know to *not* do something is the driver is older than X. MythTV, for example, used to use the driver version to work around old VBI bugs and MPEG encoder quirks that the older version of the driver may not have known how to handle: https://github.com/MythTV/mythtv/blob/b98d3a98e3187000ae652df5ffebe2beb5221ba7/mythtv/libs/libmythtv/mpegrecorder.cpp#L335 But for newer versions, MythTV could avoid using its own odd hacks. The bleeding edge MythTV now has most of these removed. Removing it is a good thing. Really, this is all about applications being able to jam a hack into their code that translates to don't call this ioctl() with some particular argument if it's driver W less than version X, because the driver had a bug that is likely to panic the guy's PC. Well, not even panics per se, but some thing like the VBI is broken, or the volume control doesn't work, IR blaster is works for this version, or something else stupid that is very visible to the end user. I also use the driver version for troubleshooting problem with users. I roughly know what wasn't working in what version of the cx18 and ivtv drivers. If the end user can tell me the driver version (using v4l2-ctl --log-status) along with his symptoms, it makes my life easier. Being able to efficiently help the end user is a win for both me and the end user. If you add it to MODULE_VERSION, you can get the version with: $ modinfo -F version vivi 0.8.1 Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Jun 24 Devin Heitmueller wrote: On Fri, Jun 24, 2011 at 5:20 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: Easier: I run Ubuntu 10.4. I run kernel 2.6.32. One of these is usually already included in the first post or IRC message from the user. Separate driver versions are only needed on platforms where drivers are not distributed by the operating system distributor, or driver source code is not released within kernel source code. Unfortunately, this doesn't work as all too often the user has Ubuntu 10.1 but I installed the latest media_build tree a few months ago. Hence they are not necessarily on a particular binary release from a distro but rather have a mix of a distro's binary release and a v4l-dvb tree compiled from source. If you release out-of-kernel-source driver sources for compilation against binary kernels, and you have got users who go through this procedure, then the user can for sure tell you the SCM version of the driver. Besides, isn't this outdated practice in times where Joe Enduser can get the very latest -rc kernel prepackaged on many distributions, including ones like Ubuntu? [Sorry, I'm getting perhaps a bit off-topic.] -- Stefan Richter -=-==-== -==- ==--= http://arcgraph.de/sr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
On Fri, 2011-06-24 at 19:16 -0300, Mauro Carvalho Chehab wrote: Em 24-06-2011 18:04, Andy Walls escreveu: On Fri, 2011-06-24 at 14:48 -0400, Devin Heitmueller wrote: On Fri, Jun 24, 2011 at 2:34 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: If the driver version is in fact an ABI version, then the driver author should really increase it only when ABI behavior is changed (and only if the behavior change can only be communicated by version number --- e.g. addition of an ioctl is not among such reasons). And the author should commit behavior changing implementation and version number change in a single changeset. And anybody who backmerges such an ABI behavior change into another kernel branch (stable, longterm, distro...) must backmerge the associated version number change too. Of course sometimes people realize this only after the fact. Or driver authors don't have a clear understanding of ABI versioning to begin with. I am saying so because I had to learn it too; I certainly wasn't born with an instinct knowledge how to do it properly. (Disclaimer: I have no stake in drivers/media/ ABIs. But I am involved in maintaining a userspace ABI elsewhere in drivers/firewire/, and one of the userspace libraries that use this ABI.) Hi Stefan, To be clear, I don't think anyone is actually proposing that the driver version number really be used as any form of formal ABI versioning scheme. In almost all cases, it's so the application can know to *not* do something is the driver is older than X. MythTV, for example, used to use the driver version to work around old VBI bugs and MPEG encoder quirks that the older version of the driver may not have known how to handle: https://github.com/MythTV/mythtv/blob/b98d3a98e3187000ae652df5ffebe2beb5221ba7/mythtv/libs/libmythtv/mpegrecorder.cpp#L335 But for newer versions, MythTV could avoid using its own odd hacks. The bleeding edge MythTV now has most of these removed. Removing it is a good thing. Really, this is all about applications being able to jam a hack into their code that translates to don't call this ioctl() with some particular argument if it's driver W less than version X, because the driver had a bug that is likely to panic the guy's PC. Well, not even panics per se, but some thing like the VBI is broken, or the volume control doesn't work, IR blaster is works for this version, or something else stupid that is very visible to the end user. I also use the driver version for troubleshooting problem with users. I roughly know what wasn't working in what version of the cx18 and ivtv drivers. If the end user can tell me the driver version (using v4l2-ctl --log-status) along with his symptoms, it makes my life easier. Being able to efficiently help the end user is a win for both me and the end user. If you add it to MODULE_VERSION, you can get the version with: $ modinfo -F version vivi 0.8.1 Well, since you mention it http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/cx18/cx18-driver.c;h=9e2f870f4258665ae6093c762f752d45147a8c98;hb=staging/for_v3.1#l252 http://git.linuxtv.org/media_tree.git?a=blob;f=drivers/media/video/ivtv/ivtv-driver.c;h=0fb75524484d909af4925c3c33c9f12cf6d6519e;hb=staging/for_v3.1#l280 However, since I often must ask for the output of v4l2-ctl --log-status, which already has the version number, I never need to ask the user to run /sbin/modinfo for the version. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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/
Em 24-06-2011 19:39, Stefan Richter escreveu: On Jun 24 Devin Heitmueller wrote: On Fri, Jun 24, 2011 at 5:20 PM, Stefan Richter stef...@s5r6.in-berlin.de wrote: Easier: I run Ubuntu 10.4. I run kernel 2.6.32. One of these is usually already included in the first post or IRC message from the user. Separate driver versions are only needed on platforms where drivers are not distributed by the operating system distributor, or driver source code is not released within kernel source code. Unfortunately, this doesn't work as all too often the user has Ubuntu 10.1 but I installed the latest media_build tree a few months ago. Hence they are not necessarily on a particular binary release from a distro but rather have a mix of a distro's binary release and a v4l-dvb tree compiled from source. If you release out-of-kernel-source driver sources for compilation against binary kernels, and you have got users who go through this procedure, then the user can for sure tell you the SCM version of the driver. Yes, and this is currently provided. The dmesg will show the last 3 git commits. A developer can just use git diff or git log to discover what changed since those commits. Besides, isn't this outdated practice in times where Joe Enduser can get the very latest -rc kernel prepackaged on many distributions, including ones like Ubuntu? Perhaps, but the cost to maintain the out-of-tree driver git tree is cheap. We provide just a small building system, with a script that downloads a daily tarball with just drivers/media and the corresponding includes (and a few drivers/staging). The building system has a couple patches to allow backport compilation since 2.6.32. Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
