[patch] video: fix build bugs in drivers/media/video/stk-webcam.c

[patch] video: fix build bugs in drivers/media/video/stk-webcam.c

[Ingo Molnar]

x86.git randconfig testing found these build bugs in current -git:

 drivers/media/video/stk-webcam.c: In function 'stk_create_sysfs_files':
 drivers/media/video/stk-webcam.c:340: error: implicit declaration of function 'video_device_create_file'
 drivers/media/video/stk-webcam.c: In function 'stk_remove_sysfs_files':
 [...]
 drivers/media/video/stk-webcam.c: In function 'v4l_stk_mmap':
 drivers/media/video/stk-webcam.c:846: error: 'VM_WRITE' undeclared (first use in this function)
 [...]

config at:

  http://redhat.com/~mingo/misc/config-Tue_Apr_29_15_00_47_CEST_2008.bad

the reason was a missing include file, and a dependency on a deprecated
(but still in use) VIDEO_V4L1_COMPAT API.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/media/video/Kconfig      |    1 +
 drivers/media/video/stk-webcam.c |    1 +
 2 files changed, 2 insertions(+)

Index: linux/drivers/media/video/Kconfig
===================================================================
--- linux.orig/drivers/media/video/Kconfig
+++ linux/drivers/media/video/Kconfig
@@ -835,6 +835,7 @@ config USB_ZR364XX
 config USB_STKWEBCAM
 	tristate "USB Syntek DC1125 Camera support"
 	depends on VIDEO_V4L2 && EXPERIMENTAL
+	select VIDEO_V4L1_COMPAT
 	---help---
 	  Say Y here if you want to use this type of camera.
 	  Supported devices are typically found in some Asus laptops,
Index: linux/drivers/media/video/stk-webcam.c
===================================================================
--- linux.orig/drivers/media/video/stk-webcam.c
+++ linux/drivers/media/video/stk-webcam.c
@@ -28,6 +28,7 @@
 #include <linux/errno.h>
 #include <linux/slab.h>
 #include <linux/kref.h>
+#include <linux/mm.h>
 
 #include <linux/usb.h>
 #include <linux/vmalloc.h>

Re: [patch] video: fix build bugs in drivers/media/video/stk-webcam.c

[Jaime Velasco]

2008/4/29 Ingo Molnar <mingo@elte.hu>:
>
>  x86.git randconfig testing found these build bugs in current -git:
>
>   drivers/media/video/stk-webcam.c: In function 'stk_create_sysfs_files':
>   drivers/media/video/stk-webcam.c:340: error: implicit declaration of function 'video_device_create_file'
>   drivers/media/video/stk-webcam.c: In function 'stk_remove_sysfs_files':
>   [...]
>   drivers/media/video/stk-webcam.c: In function 'v4l_stk_mmap':
>   drivers/media/video/stk-webcam.c:846: error: 'VM_WRITE' undeclared (first use in this function)
>   [...]
>
>  config at:
>
>   http://redhat.com/~mingo/misc/config-Tue_Apr_29_15_00_47_CEST_2008.bad
>
>  the reason was a missing include file, and a dependency on a deprecated
>  (but still in use) VIDEO_V4L1_COMPAT API.
>
>  Signed-off-by: Ingo Molnar <mingo@elte.hu>

Acked-by: Jaime Velasco Juan <jsagarribay@gmail.com>

For the record, this bug was found in march, but it seems
I forgot to CC: the patch to Mauro, Sorry about that.

http://marc.info/?l=linux-video&m=120594964524615&w=2

Regards,
Jaime

>  ---
>   drivers/media/video/Kconfig      |    1 +
>   drivers/media/video/stk-webcam.c |    1 +
>   2 files changed, 2 insertions(+)
>
>  Index: linux/drivers/media/video/Kconfig
>  ===================================================================
>  --- linux.orig/drivers/media/video/Kconfig
>  +++ linux/drivers/media/video/Kconfig
>  @@ -835,6 +835,7 @@ config USB_ZR364XX
>   config USB_STKWEBCAM
>         tristate "USB Syntek DC1125 Camera support"
>         depends on VIDEO_V4L2 && EXPERIMENTAL
>  +       select VIDEO_V4L1_COMPAT
>         ---help---
>           Say Y here if you want to use this type of camera.
>           Supported devices are typically found in some Asus laptops,
>  Index: linux/drivers/media/video/stk-webcam.c
>  ===================================================================
>  --- linux.orig/drivers/media/video/stk-webcam.c
>  +++ linux/drivers/media/video/stk-webcam.c
>  @@ -28,6 +28,7 @@
>   #include <linux/errno.h>
>   #include <linux/slab.h>
>   #include <linux/kref.h>
>  +#include <linux/mm.h>
>
>   #include <linux/usb.h>
>   #include <linux/vmalloc.h>
>

Re: [patch] video: fix build bugs in drivers/media/video/stk-webcam.c

[Mauro Carvalho Chehab]

On Wed, 30 Apr 2008 08:11:08 +0100
"Jaime Velasco" <jsagarribay@gmail.com> wrote:

> 2008/4/29 Ingo Molnar <mingo@elte.hu>:
> >
> >  x86.git randconfig testing found these build bugs in current -git:
> >
> >   drivers/media/video/stk-webcam.c: In function 'stk_create_sysfs_files':
> >   drivers/media/video/stk-webcam.c:340: error: implicit declaration of function 'video_device_create_file'
> >   drivers/media/video/stk-webcam.c: In function 'stk_remove_sysfs_files':
> >   [...]
> >   drivers/media/video/stk-webcam.c: In function 'v4l_stk_mmap':
> >   drivers/media/video/stk-webcam.c:846: error: 'VM_WRITE' undeclared (first use in this function)
> >   [...]
> >
> >  config at:
> >
> >   http://redhat.com/~mingo/misc/config-Tue_Apr_29_15_00_47_CEST_2008.bad
> >
> >  the reason was a missing include file, and a dependency on a deprecated
> >  (but still in use) VIDEO_V4L1_COMPAT API.
> >
> >  Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> Acked-by: Jaime Velasco Juan <jsagarribay@gmail.com>

The V4L1 symbols are used to help us to track what drivers are still using the
depreciated V4L1 API, or otherwise provide support for V4L1 compatibility
layer. This is not the case of stkwebcam.

So, instead of the proposed patch, I prefer just to disable the additional
non-V4L2 sysfs interface, if the flag is not enabled.

There are some discussions at the ML to create a standard to allow configuring
V4L2 controls via sysfs, but this is not standard yet. The idea is to add some
support at videodev.h using the same handlers defined by video_ioctl2.
Unfortunately, people stopped discussing this, due to the lack of time and
people to work on this.

I think this could be a nice addition to V4L2, and will help to avoid creating
non-standardized interfaces at sysfs.

> For the record, this bug was found in march, but it seems
> I forgot to CC: the patch to Mauro, Sorry about that.
> 
> http://marc.info/?l=linux-video&m=120594964524615&w=2

I probably missed this.

---

Instead of Ingo's approach, this seems to be cleaner:

Fix compilation, when V4L1_COMPAT is disabled

From: Mauro Carvalho Chehab <mchehab@infradead.org>

This driver uses some sysfs helper functions that are available only for legacy
drivers. It also requires linux/mm.h.

This patch fixes compiliation when not in compat mode.

Thanks to Ingo Molnar for identifying this issue.

Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
---

 drivers/media/video/stk-webcam.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)


diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
index 9276ed9..b12c60c 100644
--- a/drivers/media/video/stk-webcam.c
+++ b/drivers/media/video/stk-webcam.c
@@ -30,6 +30,7 @@
 #include <linux/kref.h>
 
 #include <linux/usb.h>
+#include <linux/mm.h>
 #include <linux/vmalloc.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-common.h>
@@ -245,6 +246,8 @@ static int stk_initialise(struct stk_camera *dev)
 		return -1;
 }
 
+#ifdef CONFIG_VIDEO_V4L1_COMPAT
+
 /* sysfs functions */
 /*FIXME cleanup this */
 
@@ -350,6 +353,10 @@ static void stk_remove_sysfs_files(struct video_device *vdev)
 	video_device_remove_file(vdev, &dev_attr_vflip);
 }
 
+#else
+#define stk_create_sysfs_files(a)
+#define stk_remove_sysfs_files(a)
+#endif
 
 /* *********************************************** */
 /*





Cheers,
Mauro

Re: [patch] video: fix build bugs in drivers/media/video/stk-webcam.c

[Jaime Velasco Juan]

El mié. 07 de may. de 2008, a las 16:29:50 -0300, Mauro Carvalho Chehab escribió:
> The V4L1 symbols are used to help us to track what drivers are still using the
> depreciated V4L1 API, or otherwise provide support for V4L1 compatibility
> layer. This is not the case of stkwebcam.
> 
> So, instead of the proposed patch, I prefer just to disable the additional
> non-V4L2 sysfs interface, if the flag is not enabled.

Yeah, that might be the best solution to this problem.

> 
> There are some discussions at the ML to create a standard to allow configuring
> V4L2 controls via sysfs, but this is not standard yet. The idea is to add some
> support at videodev.h using the same handlers defined by video_ioctl2.
> Unfortunately, people stopped discussing this, due to the lack of time and
> people to work on this.
> 
> I think this could be a nice addition to V4L2, and will help to avoid creating
> non-standardized interfaces at sysfs.
> 
> > For the record, this bug was found in march, but it seems
> > I forgot to CC: the patch to Mauro, Sorry about that.
> > 
> > http://marc.info/?l=linux-video&m=120594964524615&w=2
> 
> I probably missed this.
> 
> ---
> 
> Instead of Ingo's approach, this seems to be cleaner:
> 
> Fix compilation, when V4L1_COMPAT is disabled
> 
> From: Mauro Carvalho Chehab <mchehab@infradead.org>
> 
> This driver uses some sysfs helper functions that are available only for legacy
> drivers. It also requires linux/mm.h.
> 
> This patch fixes compiliation when not in compat mode.
> 
> Thanks to Ingo Molnar for identifying this issue.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@infradead.org>
Acked-by: Jaime Velasco Juan <jsagarribay@gmail.com>

Regards

> ---
> 
>  drivers/media/video/stk-webcam.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> 
> diff --git a/drivers/media/video/stk-webcam.c b/drivers/media/video/stk-webcam.c
> index 9276ed9..b12c60c 100644
> --- a/drivers/media/video/stk-webcam.c
> +++ b/drivers/media/video/stk-webcam.c
> @@ -30,6 +30,7 @@
>  #include <linux/kref.h>
>  
>  #include <linux/usb.h>
> +#include <linux/mm.h>
>  #include <linux/vmalloc.h>
>  #include <linux/videodev2.h>
>  #include <media/v4l2-common.h>
> @@ -245,6 +246,8 @@ static int stk_initialise(struct stk_camera *dev)
>  		return -1;
>  }
>  
> +#ifdef CONFIG_VIDEO_V4L1_COMPAT
> +
>  /* sysfs functions */
>  /*FIXME cleanup this */
>  
> @@ -350,6 +353,10 @@ static void stk_remove_sysfs_files(struct video_device *vdev)
>  	video_device_remove_file(vdev, &dev_attr_vflip);
>  }
>  
> +#else
> +#define stk_create_sysfs_files(a)
> +#define stk_remove_sysfs_files(a)
> +#endif
>  
>  /* *********************************************** */
>  /*
> 
> 
> 
> 
> 
> Cheers,
> Mauro