Re: [PATCH] [media] Stop using linux/version.h on most drivers
Hi Mauro, On Saturday 25 June 2011 14:14:50 Mauro Carvalho Chehab wrote: Em 25-06-2011 07:09, Hans Verkuil escreveu: On Friday, June 24, 2011 20:25:26 Mauro Carvalho Chehab wrote: [snip] - uvcvideo: not sure about its current status about its migration to video_ioctl2. If Laurent is not doing a migration to video_ioctl2 any time soon, a simple patch for it could make it act consistently. In any case, I don't think we should create a include/media/version.h just due to uvcvideo. That's not on my todo-list for now, -EBUSY :-) I bump the uvcvideo driver version whenever the userspace API changes. Using the kernel version number applications could find out whether the API they need is implemented, but detecting API changes would become more difficult. That might not be a real issue though, as applications usually don't care. -- Regards, Laurent Pinchart -- 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: [PATCH] [media] Stop using linux/version.h on most drivers
On Friday, June 24, 2011 20:25:26 Mauro Carvalho Chehab wrote: All the modified drivers didn't have any version increment since Jan, 1 2011. Several of them didn't have any version increment for a long time, even having new features and important bug fixes happening. As we're now filling the QUERYCAP version with the current Kernel Release, we don't need to maintain a per-driver version control anymore. So, let's just use the default. In order to preserve the Kernel module version history, a KERNEL_VERSION() macro were added to all modified drivers, and the extraver number were incremented. I opted to preserve the per-driver version control to a few drivers: cx18, davinci, fsl-viu, gspca, ivtv, m5mols, soc_camera, pwc, s2255, s5p-fimc and sh_vou. The rationale is that the per-driver version control seems to be actively maintained on those. A few drivers are still using the legacy way to handle ioctl's. So, we can't do such change on them, otherwise, they'll break. Those are: uvc, pvrusb2, et61x251 and sn9c102. Yet, I think that the better for them would be to just use the default version numbering, instead of doing that by themselves. While here, removed a few not needed include linux/version.h. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Sorry, but I have to say NACK to this. If we do this, then we should do this consistently. I thought it over and filling it with the current kernel version would work well with one exception: the pwc driver has a major number of 10 which is larger than the kernel's major number. (cpia2 has a version of 3.0.0, so that just works). I am inclined to sort of close my eyes for that one and just replace it as well, but that's just me :-) The only thing that needs to be done is that media_build needs some hack to set the version field to the source git tree kernel version instead of the target's kernel version. To simplify that and to accomodate the four ioctl-legacy drivers we can make a simple include/media/version.h header that defines a V4L2_API_VERSION define. An alternative is to just add an api_version field to v4l2_querycap. That would work fine too. One reason for doing that may be to help out-of-tree drivers: for those a driver version *does* make sense. I know, we shouldn't have to care about out-of-tree drivers, but on the other hand why make life hard for them without a good reason? The more I think about it, the more I like the idea of an api_version field. It would keep pwc happy, it wouldn't require many changes to drivers, and it will not affect out-of-tree drivers. Regards, Hans --- Note: This patch assumes that cap-version = LINUX_VERSION_CODE is filled inside v4l2-ioctl [1] [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg33547.html drivers/media/video/arv.c |5 ++--- drivers/media/video/au0828/au0828-core.c|1 + drivers/media/video/au0828/au0828-video.c |5 - drivers/media/video/bt8xx/bttv-driver.c | 14 -- drivers/media/video/bt8xx/bttvp.h |3 --- drivers/media/video/bw-qcam.c |3 +-- drivers/media/video/c-qcam.c|3 +-- drivers/media/video/cpia2/cpia2.h |5 - drivers/media/video/cpia2/cpia2_v4l.c | 12 drivers/media/video/cx231xx/cx231xx-video.c | 14 -- drivers/media/video/cx231xx/cx231xx.h |1 - drivers/media/video/cx23885/altera-ci.c |1 - drivers/media/video/cx23885/cx23885-417.c |1 - drivers/media/video/cx23885/cx23885-core.c | 13 +++-- drivers/media/video/cx23885/cx23885-video.c |1 - drivers/media/video/cx23885/cx23885.h |3 +-- drivers/media/video/cx88/cx88-alsa.c| 19 --- drivers/media/video/cx88/cx88-blackbird.c | 20 +++- drivers/media/video/cx88/cx88-dvb.c | 18 +++--- drivers/media/video/cx88/cx88-mpeg.c| 11 +++ drivers/media/video/cx88/cx88-video.c | 21 +++-- drivers/media/video/cx88/cx88.h |4 ++-- drivers/media/video/em28xx/em28xx-video.c | 14 +- drivers/media/video/gspca/gl860/gl860.h |1 - drivers/media/video/hdpvr/hdpvr-core.c |1 + drivers/media/video/hdpvr/hdpvr-video.c |2 -- drivers/media/video/hdpvr/hdpvr.h |6 -- drivers/media/video/mem2mem_testdev.c |4 +--- drivers/media/video/pms.c |4 +--- drivers/media/video/pwc/pwc-ioctl.h |1 - drivers/media/video/pwc/pwc.h |8 drivers/media/video/saa7134/saa7134-core.c | 12 drivers/media/video/saa7134/saa7134-empress.c |1 -
Re: [PATCH] [media] Stop using linux/version.h on most drivers
Em 25-06-2011 07:09, Hans Verkuil escreveu: On Friday, June 24, 2011 20:25:26 Mauro Carvalho Chehab wrote: All the modified drivers didn't have any version increment since Jan, 1 2011. Several of them didn't have any version increment for a long time, even having new features and important bug fixes happening. As we're now filling the QUERYCAP version with the current Kernel Release, we don't need to maintain a per-driver version control anymore. So, let's just use the default. In order to preserve the Kernel module version history, a KERNEL_VERSION() macro were added to all modified drivers, and the extraver number were incremented. I opted to preserve the per-driver version control to a few drivers: cx18, davinci, fsl-viu, gspca, ivtv, m5mols, soc_camera, pwc, s2255, s5p-fimc and sh_vou. The rationale is that the per-driver version control seems to be actively maintained on those. A few drivers are still using the legacy way to handle ioctl's. So, we can't do such change on them, otherwise, they'll break. Those are: uvc, pvrusb2, et61x251 and sn9c102. Yet, I think that the better for them would be to just use the default version numbering, instead of doing that by themselves. While here, removed a few not needed include linux/version.h. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Sorry, but I have to say NACK to this. If we do this, then we should do this consistently. IMO, all drivers should stop reporting its own version via V4L2 API: people forget to maintain it on a consistent way. That was my original proposal, and it can still be implemented later. This patch does it on a consistent way: on places were version is not updated for more than 6 months (2 kernel releases) and use video_ioctl2, the version string were replaced by a number that it is greater than the previous value. My original proposal were to replace it on every place, but you and Devin argued against. So, this approach is a mid-term: let's do it initially where it makes more sense, and discuss what will be done with the remaining drivers that implement their own version control. With the mid-term approach taken by this patch, we warrant that, when newer V4L2 core API changes are applied, the version number will also be incremented, reflecting that changes could have affected the driver. I thought it over and filling it with the current kernel version would work well with one exception: the pwc driver has a major number of 10 which is larger than the kernel's major number. (cpia2 has a version of 3.0.0, so that just works). That's what I've explained on my first email about this subject: the only driver that has a numbering 3.0.0 is the pwc driver, and it does that on a not consistent way: when you've removed the V4L1 API, you've incremented the version string, but you forgot to upgrade the caps-version. So, if any application is relying on it for pwc, the check is currently broken. I am inclined to sort of close my eyes for that one and just replace it as well, but that's just me :-) I like the idea of replacing pwc version to 3.0.0. We moved it into drivers/media at 2006-03-25. On that time, the version was: 9.0.2-unofficial, and only the V4L1 API was implemented. In other words, the pwc driver never returned version 3.0.0 to VIDIOC_QUERYCAP. So, it is safe to just use 3.x.y. Assuming that we'll be incrementing Kernel major versions on every 10 years, the namespace conflicts will happen 70 years from now ;) Seriously, I don't think we need to be concerned with a conflict with 10.0.x. numbering that will happen on lots of years in the future (also, because we probably won't use the extraver numbering). The only thing that needs to be done is that media_build needs some hack to set the version field to the source git tree kernel version instead of the target's kernel version. Yes. After applying those patches, we'll need to fix the out-of-tree media_build. It shouldn't be hard: a patch can just add a logic that uses a V4L2_VERSION version defined on v4l/v4l2-version.h, for example. To simplify that and to accomodate the four ioctl-legacy drivers we can make a simple include/media/version.h header that defines a V4L2_API_VERSION define. I would not care much about those. The current situation is: - et61x251: only one USB ID is currently supported there: 102c:6251, using the TAS5130D1B sensor. The gspca etoms driver supports the same USB ID with the same sensor. IMO, we can just move this driver to staging and remove it. - sn9c102: I think that there are still a few USB ID's there that aren't yet at the gspca/sonix* drivers, but Jean-François/Hans could give us a better status. Anyway, no new features or bug fixes are added there for a long time, and core changes should likely not affect this driver, as it doesn't use the subdev layer nor video_ioctl2. So, we can just keep its version there until its removal; - pvrusb2:
[PATCH] [media] Stop using linux/version.h on most drivers
All the modified drivers didn't have any version increment since Jan, 1 2011. Several of them didn't have any version increment for a long time, even having new features and important bug fixes happening. As we're now filling the QUERYCAP version with the current Kernel Release, we don't need to maintain a per-driver version control anymore. So, let's just use the default. In order to preserve the Kernel module version history, a KERNEL_VERSION() macro were added to all modified drivers, and the extraver number were incremented. I opted to preserve the per-driver version control to a few drivers: cx18, davinci, fsl-viu, gspca, ivtv, m5mols, soc_camera, pwc, s2255, s5p-fimc and sh_vou. The rationale is that the per-driver version control seems to be actively maintained on those. A few drivers are still using the legacy way to handle ioctl's. So, we can't do such change on them, otherwise, they'll break. Those are: uvc, pvrusb2, et61x251 and sn9c102. Yet, I think that the better for them would be to just use the default version numbering, instead of doing that by themselves. While here, removed a few not needed include linux/version.h. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com --- Note: This patch assumes that cap-version = LINUX_VERSION_CODE is filled inside v4l2-ioctl [1] [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg33547.html drivers/media/video/arv.c |5 ++--- drivers/media/video/au0828/au0828-core.c|1 + drivers/media/video/au0828/au0828-video.c |5 - drivers/media/video/bt8xx/bttv-driver.c | 14 -- drivers/media/video/bt8xx/bttvp.h |3 --- drivers/media/video/bw-qcam.c |3 +-- drivers/media/video/c-qcam.c|3 +-- drivers/media/video/cpia2/cpia2.h |5 - drivers/media/video/cpia2/cpia2_v4l.c | 12 drivers/media/video/cx231xx/cx231xx-video.c | 14 -- drivers/media/video/cx231xx/cx231xx.h |1 - drivers/media/video/cx23885/altera-ci.c |1 - drivers/media/video/cx23885/cx23885-417.c |1 - drivers/media/video/cx23885/cx23885-core.c | 13 +++-- drivers/media/video/cx23885/cx23885-video.c |1 - drivers/media/video/cx23885/cx23885.h |3 +-- drivers/media/video/cx88/cx88-alsa.c| 19 --- drivers/media/video/cx88/cx88-blackbird.c | 20 +++- drivers/media/video/cx88/cx88-dvb.c | 18 +++--- drivers/media/video/cx88/cx88-mpeg.c| 11 +++ drivers/media/video/cx88/cx88-video.c | 21 +++-- drivers/media/video/cx88/cx88.h |4 ++-- drivers/media/video/em28xx/em28xx-video.c | 14 +- drivers/media/video/gspca/gl860/gl860.h |1 - drivers/media/video/hdpvr/hdpvr-core.c |1 + drivers/media/video/hdpvr/hdpvr-video.c |2 -- drivers/media/video/hdpvr/hdpvr.h |6 -- drivers/media/video/mem2mem_testdev.c |4 +--- drivers/media/video/pms.c |4 +--- drivers/media/video/pwc/pwc-ioctl.h |1 - drivers/media/video/pwc/pwc.h |8 drivers/media/video/saa7134/saa7134-core.c | 12 drivers/media/video/saa7134/saa7134-empress.c |1 - drivers/media/video/saa7134/saa7134-video.c |2 -- drivers/media/video/saa7134/saa7134.h |3 +-- drivers/media/video/saa7164/saa7164.h |1 - drivers/media/video/timblogiw.c |1 - drivers/media/video/tlg2300/pd-common.h |1 - drivers/media/video/tlg2300/pd-main.c |1 + drivers/media/video/tlg2300/pd-radio.c |2 -- drivers/media/video/usbvision/usbvision-video.c | 12 +--- drivers/media/video/vino.c |5 + drivers/media/video/vivi.c | 14 -- drivers/media/video/w9966.c |4 +--- drivers/media/video/zoran/zoran.h |4 drivers/media/video/zoran/zoran_card.c |7 +-- drivers/media/video/zoran/zoran_driver.c|3 --- drivers/media/video/zr364xx.c |6 ++ 48 files changed, 71 insertions(+), 227 deletions(-) diff --git a/drivers/media/video/arv.c b/drivers/media/video/arv.c index f989f28..b6ed44a 100644 --- a/drivers/media/video/arv.c +++ b/drivers/media/video/arv.c @@ -27,7 +27,6 @@ #include linux/slab.h #include linux/mm.h #include linux/sched.h -#include linux/version.h #include linux/videodev2.h #include media/v4l2-common.h #include media/v4l2-device.h @@ -54,7 +53,7 @@ */ #define USE_INT0 /* Don't modify */ -#define VERSION0.04 +#define VERSION0.0.5 #define
