Re: [PATCH] v4l: fix build error for et61x251 driver
through ioctl()? It's not as immediate and safe as controlling the device registers through /sysfs (not /proc). However, the sysfs interface in those drivers appeared before V4L2 had its own ioctls and we agreed to keep and export the interface to the only users selecting CONFIG_VIDEO_ADV_DEBUG (ok, this is actually valid for the sn9c102, I'll submit a patch for the et61x251 in the future). Direct register access for debug ok, but not this is not ok for normal usage. From my POV, a driver that is creating its own userspace API is not fully compliant with V4L2 API. Isn't Video4Linux2 ioctl() supersedeing sysfs in this case? It should be. However, things like direct register access (for non-debug mode) may allow some controls that weren't visible via ioctl. That's why the sysfs usage may be evil: a driver may have some parts accessible only via sysfs interface, on a non-standard way, without offering the official API support. So, some device functionalities may be hidden from userspace apps that are compliant with V4L2. Summarizing: Linus patch seems to be the better solution to solve the V4L1_COMPAT bug. I would also convert the other container_of stuff to to_video_device (to have code uniformity). et61x251 direct register interfaces should be available only if CONFIG_VIDEO_ADV_DEBUG is selected to avoid its miss-usage. If there are other device configurations that needs specific register settings not yet provided, this should be provided via V4L2 standard ioctls. This way, et61x251 will be compliant with V4L2. I still think that we should work at the remaining sysfs classes to make them coherent on all V4L devices. Cheers, Mauro. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
On Friday 14 September 2007 14:50:11 Mauro Carvalho Chehab wrote: > > > > This patch is really ugly. > > > > Well, yes. I should have known as I converted so many occurences of > > to_video_device to container_of in my second patch. > > > > > > Why can't the "to_video_device()" macro be used? Just move it to a > > > > place where it's usable! IOW, what's wrong with the *much* simpler > > > > patch below? > > > > > > There's nothing wtong in my opinion. I do not know the exact reason why > > > Mauro moved "to_video_device()" into CONFIG_VIDEO_V4L1_COMPAT. Pheraps > > > he can give more details about this change. > > > > BTW, just a few V4L2 drivers and videodev seem to use this construct: > > video/usbvision/usbvision-video.c:container_of(cd, struct > > video_device, class_dev); > > > > video/sn9c102/sn9c102_core.c: cam = > > video_get_drvdata(container_of(cd, struct video_device, > > video/sn9c102/sn9c102_core.c- > > class_dev)); > > > > video/videodev.c: struct video_device *vfd = container_of(cd, > > struct video_device, video/videodev.c- > >class_dev); > > > > And then their are drivers with other variants of container_of, e.g.: > > video/pvrusb2/pvrusb2-v4l2.c: vp = container_of(chp,struct > > pvr2_v4l2,channel); video/bt8xx/bttv-vbi.c: struct bttv_buffer *buf = > > container_of(vb,struct bttv_buffer,vb); ... > > > > I think, there should be a to_video_device macro for usage in v4l2. > > An most probable for the other container_of stuff (when more there is > > more than one occurence of a particular construct), drivers should > > provide their own macro, e.g. to_video_buffer() or so. > > > > That's what other subsystems do. It is more self-explanatory than a > > direct usage of container_of. > > > > So why not apply (my first patch ... oh no, of course that's rubbish ;-) > > Linus' below patch for 2.6.23 to fix the compilation for that particular > > driver. And to work on the conversion of container_of() stuff to > > meaningful macros for the next kernel release? > > The to_video_device and the container_of (cd, struct video_device, > class_dev) (as you noticed at the above drivers) are used to provide > procfs interface. > > On videodev, there's the v4l2 core stuff, used on all V4L drivers. It > allows some control to the V4L devices (basically, see/change the > modprobe loading parameters). > > The other drivers that uses to_video_device (or the container_of > alternative) to create other userspace interfaces, specific to each > driver and not documented at V4L2 API: > > bttv-driver.c:static CLASS_DEVICE_ATTR(card, S_IRUGO, show_card, NULL); > et61x251_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, > et61x251_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, > et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, > et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, > ov511.c:static CLASS_DEVICE_ATTR(custom_id, S_IRUGO, show_custom_id, NULL); > ov511.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); > ov511.c:static CLASS_DEVICE_ATTR(bridge, S_IRUGO, show_bridge, NULL); > ov511.c:static CLASS_DEVICE_ATTR(sensor, S_IRUGO, show_sensor, NULL); > ov511.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, > NULL); ov511.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, > show_saturation, NULL); ov511.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, > show_contrast, NULL); ov511.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, > show_hue, NULL); > ov511.c:static CLASS_DEVICE_ATTR(exposure, S_IRUGO, show_exposure, NULL); > pwc-if.c:static CLASS_DEVICE_ATTR(pan_tilt, S_IRUGO | S_IWUSR, > show_pan_tilt, pwc-if.c:static CLASS_DEVICE_ATTR(button, S_IRUGO | S_IWUSR, > show_snapshot_button_status, sn9c102_core.c:static CLASS_DEVICE_ATTR(reg, > S_IRUGO | S_IWUSR, > sn9c102_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, > sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, > sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, > sn9c102_core.c:static CLASS_DEVICE_ATTR(green, S_IWUGO, NULL, > sn9c102_store_green); sn9c102_core.c:static CLASS_DEVICE_ATTR(blue, > S_IWUGO, NULL, sn9c102_store_blue); sn9c102_core.c:static > CLASS_DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red); > sn9c102_core.c:static CLASS_DEVICE_ATTR(frame_header, S_IRUGO, > stv680.c:static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL); > usbvision-video.c:static CLASS_DEVICE_ATTR(version, S_IRUGO, show_version, > NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, > show_model, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, > show_hue, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(contrast, > S_IRUGO, show_contrast, NULL); usbvision-video.c:static > CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); > usbvision-video.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, > show_saturation, NULL);
Re: [PATCH] v4l: fix build error for et61x251 driver
> > > This patch is really ugly. > > Well, yes. I should have known as I converted so many occurences of > to_video_device to container_of in my second patch. > > > > Why can't the "to_video_device()" macro be used? Just move it to a place > > > where it's usable! IOW, what's wrong with the *much* simpler patch below? > > > > There's nothing wtong in my opinion. I do not know the exact reason why > > Mauro > > moved "to_video_device()" into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can > > give > > more details about this change. > > BTW, just a few V4L2 drivers and videodev seem to use this construct: > video/usbvision/usbvision-video.c:container_of(cd, struct video_device, > class_dev); > > video/sn9c102/sn9c102_core.c: cam = video_get_drvdata(container_of(cd, > struct video_device, > video/sn9c102/sn9c102_core.c- > class_dev)); > > video/videodev.c: struct video_device *vfd = container_of(cd, struct > video_device, > video/videodev.c- class_dev); > > And then their are drivers with other variants of container_of, e.g.: > video/pvrusb2/pvrusb2-v4l2.c: vp = container_of(chp,struct > pvr2_v4l2,channel); > video/bt8xx/bttv-vbi.c: struct bttv_buffer *buf = container_of(vb,struct > bttv_buffer,vb); >... > > I think, there should be a to_video_device macro for usage in v4l2. > An most probable for the other container_of stuff (when more there is more > than one occurence of a particular construct), drivers should provide their > own macro, > e.g. to_video_buffer() or so. > > That's what other subsystems do. It is more self-explanatory than a direct > usage > of container_of. > > So why not apply (my first patch ... oh no, of course that's rubbish ;-) > Linus' below patch for 2.6.23 to fix the compilation for that particular > driver. > And to work on the conversion of container_of() stuff to meaningful macros > for the > next kernel release? The to_video_device and the container_of (cd, struct video_device, class_dev) (as you noticed at the above drivers) are used to provide procfs interface. On videodev, there's the v4l2 core stuff, used on all V4L drivers. It allows some control to the V4L devices (basically, see/change the modprobe loading parameters). The other drivers that uses to_video_device (or the container_of alternative) to create other userspace interfaces, specific to each driver and not documented at V4L2 API: bttv-driver.c:static CLASS_DEVICE_ATTR(card, S_IRUGO, show_card, NULL); et61x251_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, ov511.c:static CLASS_DEVICE_ATTR(custom_id, S_IRUGO, show_custom_id, NULL); ov511.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); ov511.c:static CLASS_DEVICE_ATTR(bridge, S_IRUGO, show_bridge, NULL); ov511.c:static CLASS_DEVICE_ATTR(sensor, S_IRUGO, show_sensor, NULL); ov511.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); ov511.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL); ov511.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL); ov511.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL); ov511.c:static CLASS_DEVICE_ATTR(exposure, S_IRUGO, show_exposure, NULL); pwc-if.c:static CLASS_DEVICE_ATTR(pan_tilt, S_IRUGO | S_IWUSR, show_pan_tilt, pwc-if.c:static CLASS_DEVICE_ATTR(button, S_IRUGO | S_IWUSR, show_snapshot_button_status, sn9c102_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(green, S_IWUGO, NULL, sn9c102_store_green); sn9c102_core.c:static CLASS_DEVICE_ATTR(blue, S_IWUGO, NULL, sn9c102_store_blue); sn9c102_core.c:static CLASS_DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red); sn9c102_core.c:static CLASS_DEVICE_ATTR(frame_header, S_IRUGO, stv680.c:static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(version, S_IRUGO, show_version, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(streaming, S_IRUGO, show_streaming, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(compression, S_IRUGO, show_compression, NULL);
Re: [PATCH] v4l: fix build error for et61x251 driver
On Fri, Sep 14, 2007 at 03:53:06AM +0200, Luca Risolia wrote: > On Friday 14 September 2007 02:09:01 Linus Torvalds wrote: > > On Fri, 14 Sep 2007, Luca Risolia wrote: > > > Hacked-by: Luca Risolia <[EMAIL PROTECTED]> > > > > > > On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote: > > > > This fixes a kernel build problem and > > > > should make it into 2.6.23, I think. > > > > > > > > > > This patch is really ugly. Well, yes. I should have known as I converted so many occurences of to_video_device to container_of in my second patch. > > Why can't the "to_video_device()" macro be used? Just move it to a place > > where it's usable! IOW, what's wrong with the *much* simpler patch below? > > There's nothing wtong in my opinion. I do not know the exact reason why Mauro > moved "to_video_device()" into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can give > more details about this change. BTW, just a few V4L2 drivers and videodev seem to use this construct: video/usbvision/usbvision-video.c:container_of(cd, struct video_device, class_dev); video/sn9c102/sn9c102_core.c: cam = video_get_drvdata(container_of(cd, struct video_device, video/sn9c102/sn9c102_core.c- class_dev)); video/videodev.c: struct video_device *vfd = container_of(cd, struct video_device, video/videodev.c- class_dev); And then their are drivers with other variants of container_of, e.g.: video/pvrusb2/pvrusb2-v4l2.c: vp = container_of(chp,struct pvr2_v4l2,channel); video/bt8xx/bttv-vbi.c: struct bttv_buffer *buf = container_of(vb,struct bttv_buffer,vb); ... I think, there should be a to_video_device macro for usage in v4l2. An most probable for the other container_of stuff (when more there is more than one occurence of a particular construct), drivers should provide their own macro, e.g. to_video_buffer() or so. That's what other subsystems do. It is more self-explanatory than a direct usage of container_of. So why not apply (my first patch ... oh no, of course that's rubbish ;-) Linus' below patch for 2.6.23 to fix the compilation for that particular driver. And to work on the conversion of container_of() stuff to meaningful macros for the next kernel release? Regards, Andreas > > > > That "to_video_device()" macro has absolutely _nothing_ to do with > > CONFIG_VIDEO_V4L1_COMPAT, as far as I can tell! > > > > Linus > > --- > > diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h > > index d62847f..17f8f3a 100644 > > --- a/include/media/v4l2-dev.h > > +++ b/include/media/v4l2-dev.h > > @@ -337,6 +337,9 @@ void *priv; > > struct class_device class_dev; /* sysfs */ > > }; > > > > +/* Class-dev to video-device */ > > +#define to_video_device(cd) container_of(cd, struct video_device, > > class_dev) + > > /* Version 2 functions */ > > extern int video_register_device(struct video_device *vfd, int type, int > > nr); void video_unregister_device(struct video_device *); > > @@ -354,11 +357,9 @@ extern int video_usercopy(struct inode *inode, struct > > file *file, int (*func)(struct inode *inode, struct file *file, > > unsigned int cmd, void *arg)); > > > > - > > #ifdef CONFIG_VIDEO_V4L1_COMPAT > > #include > > > > -#define to_video_device(cd) container_of(cd, struct video_device, > > class_dev) static inline int __must_check > > video_device_create_file(struct video_device *vfd, > > struct class_device_attribute *attr) > > Best regards > Luca Risolia - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
On Fri, Sep 14, 2007 at 03:53:06AM +0200, Luca Risolia wrote: On Friday 14 September 2007 02:09:01 Linus Torvalds wrote: On Fri, 14 Sep 2007, Luca Risolia wrote: Hacked-by: Luca Risolia [EMAIL PROTECTED] On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote: This fixes a kernel build problem and should make it into 2.6.23, I think. This patch is really ugly. Well, yes. I should have known as I converted so many occurences of to_video_device to container_of in my second patch. Why can't the to_video_device() macro be used? Just move it to a place where it's usable! IOW, what's wrong with the *much* simpler patch below? There's nothing wtong in my opinion. I do not know the exact reason why Mauro moved to_video_device() into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can give more details about this change. BTW, just a few V4L2 drivers and videodev seem to use this construct: video/usbvision/usbvision-video.c:container_of(cd, struct video_device, class_dev); video/sn9c102/sn9c102_core.c: cam = video_get_drvdata(container_of(cd, struct video_device, video/sn9c102/sn9c102_core.c- class_dev)); video/videodev.c: struct video_device *vfd = container_of(cd, struct video_device, video/videodev.c- class_dev); And then their are drivers with other variants of container_of, e.g.: video/pvrusb2/pvrusb2-v4l2.c: vp = container_of(chp,struct pvr2_v4l2,channel); video/bt8xx/bttv-vbi.c: struct bttv_buffer *buf = container_of(vb,struct bttv_buffer,vb); ... I think, there should be a to_video_device macro for usage in v4l2. An most probable for the other container_of stuff (when more there is more than one occurence of a particular construct), drivers should provide their own macro, e.g. to_video_buffer() or so. That's what other subsystems do. It is more self-explanatory than a direct usage of container_of. So why not apply (my first patch ... oh no, of course that's rubbish ;-) Linus' below patch for 2.6.23 to fix the compilation for that particular driver. And to work on the conversion of container_of() stuff to meaningful macros for the next kernel release? Regards, Andreas That to_video_device() macro has absolutely _nothing_ to do with CONFIG_VIDEO_V4L1_COMPAT, as far as I can tell! Linus --- diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index d62847f..17f8f3a 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -337,6 +337,9 @@ void *priv; struct class_device class_dev; /* sysfs */ }; +/* Class-dev to video-device */ +#define to_video_device(cd) container_of(cd, struct video_device, class_dev) + /* Version 2 functions */ extern int video_register_device(struct video_device *vfd, int type, int nr); void video_unregister_device(struct video_device *); @@ -354,11 +357,9 @@ extern int video_usercopy(struct inode *inode, struct file *file, int (*func)(struct inode *inode, struct file *file, unsigned int cmd, void *arg)); - #ifdef CONFIG_VIDEO_V4L1_COMPAT #include linux/mm.h -#define to_video_device(cd) container_of(cd, struct video_device, class_dev) static inline int __must_check video_device_create_file(struct video_device *vfd, struct class_device_attribute *attr) Best regards Luca Risolia - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
This patch is really ugly. Well, yes. I should have known as I converted so many occurences of to_video_device to container_of in my second patch. Why can't the to_video_device() macro be used? Just move it to a place where it's usable! IOW, what's wrong with the *much* simpler patch below? There's nothing wtong in my opinion. I do not know the exact reason why Mauro moved to_video_device() into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can give more details about this change. BTW, just a few V4L2 drivers and videodev seem to use this construct: video/usbvision/usbvision-video.c:container_of(cd, struct video_device, class_dev); video/sn9c102/sn9c102_core.c: cam = video_get_drvdata(container_of(cd, struct video_device, video/sn9c102/sn9c102_core.c- class_dev)); video/videodev.c: struct video_device *vfd = container_of(cd, struct video_device, video/videodev.c- class_dev); And then their are drivers with other variants of container_of, e.g.: video/pvrusb2/pvrusb2-v4l2.c: vp = container_of(chp,struct pvr2_v4l2,channel); video/bt8xx/bttv-vbi.c: struct bttv_buffer *buf = container_of(vb,struct bttv_buffer,vb); ... I think, there should be a to_video_device macro for usage in v4l2. An most probable for the other container_of stuff (when more there is more than one occurence of a particular construct), drivers should provide their own macro, e.g. to_video_buffer() or so. That's what other subsystems do. It is more self-explanatory than a direct usage of container_of. So why not apply (my first patch ... oh no, of course that's rubbish ;-) Linus' below patch for 2.6.23 to fix the compilation for that particular driver. And to work on the conversion of container_of() stuff to meaningful macros for the next kernel release? The to_video_device and the container_of (cd, struct video_device, class_dev) (as you noticed at the above drivers) are used to provide procfs interface. On videodev, there's the v4l2 core stuff, used on all V4L drivers. It allows some control to the V4L devices (basically, see/change the modprobe loading parameters). The other drivers that uses to_video_device (or the container_of alternative) to create other userspace interfaces, specific to each driver and not documented at V4L2 API: bttv-driver.c:static CLASS_DEVICE_ATTR(card, S_IRUGO, show_card, NULL); et61x251_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, ov511.c:static CLASS_DEVICE_ATTR(custom_id, S_IRUGO, show_custom_id, NULL); ov511.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); ov511.c:static CLASS_DEVICE_ATTR(bridge, S_IRUGO, show_bridge, NULL); ov511.c:static CLASS_DEVICE_ATTR(sensor, S_IRUGO, show_sensor, NULL); ov511.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); ov511.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL); ov511.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL); ov511.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL); ov511.c:static CLASS_DEVICE_ATTR(exposure, S_IRUGO, show_exposure, NULL); pwc-if.c:static CLASS_DEVICE_ATTR(pan_tilt, S_IRUGO | S_IWUSR, show_pan_tilt, pwc-if.c:static CLASS_DEVICE_ATTR(button, S_IRUGO | S_IWUSR, show_snapshot_button_status, sn9c102_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(green, S_IWUGO, NULL, sn9c102_store_green); sn9c102_core.c:static CLASS_DEVICE_ATTR(blue, S_IWUGO, NULL, sn9c102_store_blue); sn9c102_core.c:static CLASS_DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red); sn9c102_core.c:static CLASS_DEVICE_ATTR(frame_header, S_IRUGO, stv680.c:static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(version, S_IRUGO, show_version, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(streaming, S_IRUGO, show_streaming, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(compression, S_IRUGO, show_compression, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(bridge, S_IRUGO, show_device_bridge,
Re: [PATCH] v4l: fix build error for et61x251 driver
On Friday 14 September 2007 14:50:11 Mauro Carvalho Chehab wrote: This patch is really ugly. Well, yes. I should have known as I converted so many occurences of to_video_device to container_of in my second patch. Why can't the to_video_device() macro be used? Just move it to a place where it's usable! IOW, what's wrong with the *much* simpler patch below? There's nothing wtong in my opinion. I do not know the exact reason why Mauro moved to_video_device() into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can give more details about this change. BTW, just a few V4L2 drivers and videodev seem to use this construct: video/usbvision/usbvision-video.c:container_of(cd, struct video_device, class_dev); video/sn9c102/sn9c102_core.c: cam = video_get_drvdata(container_of(cd, struct video_device, video/sn9c102/sn9c102_core.c- class_dev)); video/videodev.c: struct video_device *vfd = container_of(cd, struct video_device, video/videodev.c- class_dev); And then their are drivers with other variants of container_of, e.g.: video/pvrusb2/pvrusb2-v4l2.c: vp = container_of(chp,struct pvr2_v4l2,channel); video/bt8xx/bttv-vbi.c: struct bttv_buffer *buf = container_of(vb,struct bttv_buffer,vb); ... I think, there should be a to_video_device macro for usage in v4l2. An most probable for the other container_of stuff (when more there is more than one occurence of a particular construct), drivers should provide their own macro, e.g. to_video_buffer() or so. That's what other subsystems do. It is more self-explanatory than a direct usage of container_of. So why not apply (my first patch ... oh no, of course that's rubbish ;-) Linus' below patch for 2.6.23 to fix the compilation for that particular driver. And to work on the conversion of container_of() stuff to meaningful macros for the next kernel release? The to_video_device and the container_of (cd, struct video_device, class_dev) (as you noticed at the above drivers) are used to provide procfs interface. On videodev, there's the v4l2 core stuff, used on all V4L drivers. It allows some control to the V4L devices (basically, see/change the modprobe loading parameters). The other drivers that uses to_video_device (or the container_of alternative) to create other userspace interfaces, specific to each driver and not documented at V4L2 API: bttv-driver.c:static CLASS_DEVICE_ATTR(card, S_IRUGO, show_card, NULL); et61x251_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, et61x251_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, ov511.c:static CLASS_DEVICE_ATTR(custom_id, S_IRUGO, show_custom_id, NULL); ov511.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); ov511.c:static CLASS_DEVICE_ATTR(bridge, S_IRUGO, show_bridge, NULL); ov511.c:static CLASS_DEVICE_ATTR(sensor, S_IRUGO, show_sensor, NULL); ov511.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); ov511.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL); ov511.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL); ov511.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL); ov511.c:static CLASS_DEVICE_ATTR(exposure, S_IRUGO, show_exposure, NULL); pwc-if.c:static CLASS_DEVICE_ATTR(pan_tilt, S_IRUGO | S_IWUSR, show_pan_tilt, pwc-if.c:static CLASS_DEVICE_ATTR(button, S_IRUGO | S_IWUSR, show_snapshot_button_status, sn9c102_core.c:static CLASS_DEVICE_ATTR(reg, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(val, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_reg, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(i2c_val, S_IRUGO | S_IWUSR, sn9c102_core.c:static CLASS_DEVICE_ATTR(green, S_IWUGO, NULL, sn9c102_store_green); sn9c102_core.c:static CLASS_DEVICE_ATTR(blue, S_IWUGO, NULL, sn9c102_store_blue); sn9c102_core.c:static CLASS_DEVICE_ATTR(red, S_IWUGO, NULL, sn9c102_store_red); sn9c102_core.c:static CLASS_DEVICE_ATTR(frame_header, S_IRUGO, stv680.c:static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(version, S_IRUGO, show_version, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(model, S_IRUGO, show_model, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(hue, S_IRUGO, show_hue, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(contrast, S_IRUGO, show_contrast, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(brightness, S_IRUGO, show_brightness, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(saturation, S_IRUGO, show_saturation, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(streaming, S_IRUGO, show_streaming, NULL); usbvision-video.c:static CLASS_DEVICE_ATTR(compression,
Re: [PATCH] v4l: fix build error for et61x251 driver
through ioctl()? It's not as immediate and safe as controlling the device registers through /sysfs (not /proc). However, the sysfs interface in those drivers appeared before V4L2 had its own ioctls and we agreed to keep and export the interface to the only users selecting CONFIG_VIDEO_ADV_DEBUG (ok, this is actually valid for the sn9c102, I'll submit a patch for the et61x251 in the future). Direct register access for debug ok, but not this is not ok for normal usage. From my POV, a driver that is creating its own userspace API is not fully compliant with V4L2 API. Isn't Video4Linux2 ioctl() supersedeing sysfs in this case? It should be. However, things like direct register access (for non-debug mode) may allow some controls that weren't visible via ioctl. That's why the sysfs usage may be evil: a driver may have some parts accessible only via sysfs interface, on a non-standard way, without offering the official API support. So, some device functionalities may be hidden from userspace apps that are compliant with V4L2. Summarizing: Linus patch seems to be the better solution to solve the V4L1_COMPAT bug. I would also convert the other container_of stuff to to_video_device (to have code uniformity). et61x251 direct register interfaces should be available only if CONFIG_VIDEO_ADV_DEBUG is selected to avoid its miss-usage. If there are other device configurations that needs specific register settings not yet provided, this should be provided via V4L2 standard ioctls. This way, et61x251 will be compliant with V4L2. I still think that we should work at the remaining sysfs classes to make them coherent on all V4L devices. Cheers, Mauro. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
On Friday 14 September 2007 02:09:01 Linus Torvalds wrote: > On Fri, 14 Sep 2007, Luca Risolia wrote: > > Hacked-by: Luca Risolia <[EMAIL PROTECTED]> > > > > On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote: > > > This fixes a kernel build problem and > > > should make it into 2.6.23, I think. > > > > > > > > > Regards, > > > > > > Andreas > > > > > > -- > > > > > > Get rid of some v4l1 remainders to avoid kernel build errors if > > > V4L1_COMPAT is not selected: > > > > > > drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_: > > > drivers/media/video/et61x251/et61x251_core.c:718: error: implicit > > > declaration of to_video_device > > > > > > Fix as suggested by Luca Risolia <[EMAIL PROTECTED]> > > This patch is really ugly. > > Why can't the "to_video_device()" macro be used? Just move it to a place > where it's usable! IOW, what's wrong with the *much* simpler patch below? There's nothing wtong in my opinion. I do not know the exact reason why Mauro moved "to_video_device()" into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can give more details about this change. > That "to_video_device()" macro has absolutely _nothing_ to do with > CONFIG_VIDEO_V4L1_COMPAT, as far as I can tell! > > Linus > --- > diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h > index d62847f..17f8f3a 100644 > --- a/include/media/v4l2-dev.h > +++ b/include/media/v4l2-dev.h > @@ -337,6 +337,9 @@ void *priv; > struct class_device class_dev; /* sysfs */ > }; > > +/* Class-dev to video-device */ > +#define to_video_device(cd) container_of(cd, struct video_device, > class_dev) + > /* Version 2 functions */ > extern int video_register_device(struct video_device *vfd, int type, int > nr); void video_unregister_device(struct video_device *); > @@ -354,11 +357,9 @@ extern int video_usercopy(struct inode *inode, struct > file *file, int (*func)(struct inode *inode, struct file *file, > unsigned int cmd, void *arg)); > > - > #ifdef CONFIG_VIDEO_V4L1_COMPAT > #include > > -#define to_video_device(cd) container_of(cd, struct video_device, > class_dev) static inline int __must_check > video_device_create_file(struct video_device *vfd, >struct class_device_attribute *attr) Best regards Luca Risolia - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
On Fri, 14 Sep 2007, Luca Risolia wrote: > Hacked-by: Luca Risolia <[EMAIL PROTECTED]> > > On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote: > > This fixes a kernel build problem and > > should make it into 2.6.23, I think. > > > > > > Regards, > > > > Andreas > > > > -- > > > > Get rid of some v4l1 remainders to avoid kernel build errors if > > V4L1_COMPAT is not selected: > > > > drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_: > > drivers/media/video/et61x251/et61x251_core.c:718: error: implicit > > declaration of to_video_device > > > > Fix as suggested by Luca Risolia <[EMAIL PROTECTED]> This patch is really ugly. Why can't the "to_video_device()" macro be used? Just move it to a place where it's usable! IOW, what's wrong with the *much* simpler patch below? That "to_video_device()" macro has absolutely _nothing_ to do with CONFIG_VIDEO_V4L1_COMPAT, as far as I can tell! Linus --- diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index d62847f..17f8f3a 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -337,6 +337,9 @@ void *priv; struct class_device class_dev; /* sysfs */ }; +/* Class-dev to video-device */ +#define to_video_device(cd) container_of(cd, struct video_device, class_dev) + /* Version 2 functions */ extern int video_register_device(struct video_device *vfd, int type, int nr); void video_unregister_device(struct video_device *); @@ -354,11 +357,9 @@ extern int video_usercopy(struct inode *inode, struct file *file, int (*func)(struct inode *inode, struct file *file, unsigned int cmd, void *arg)); - #ifdef CONFIG_VIDEO_V4L1_COMPAT #include -#define to_video_device(cd) container_of(cd, struct video_device, class_dev) static inline int __must_check video_device_create_file(struct video_device *vfd, struct class_device_attribute *attr) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
Hacked-by: Luca Risolia <[EMAIL PROTECTED]>
On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote:
> This fixes a kernel build problem and
> should make it into 2.6.23, I think.
>
>
> Regards,
>
> Andreas
>
> --
>
> Get rid of some v4l1 remainders to avoid kernel build errors if
> V4L1_COMPAT is not selected:
>
> drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_:
> drivers/media/video/et61x251/et61x251_core.c:718: error: implicit
> declaration of to_video_device
>
> Fix as suggested by Luca Risolia <[EMAIL PROTECTED]>
>
> Signed-off-by: Andreas Herrmann <[EMAIL PROTECTED]>
> ---
> drivers/media/video/et61x251/et61x251_core.c | 24
> 1 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/video/et61x251/et61x251_core.c
> b/drivers/media/video/et61x251/et61x251_core.c index 585bd1f..a3ee968
> 100644
> --- a/drivers/media/video/et61x251/et61x251_core.c
> +++ b/drivers/media/video/et61x251/et61x251_core.c
> @@ -715,7 +715,8 @@ static ssize_t et61x251_show_reg(struct class_device*
> cd, char* buf) if (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -739,7 +740,8 @@ et61x251_store_reg(struct class_device* cd, const char*
> buf, size_t len) if (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -771,7 +773,8 @@ static ssize_t et61x251_show_val(struct class_device*
> cd, char* buf) if (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -803,7 +806,8 @@ et61x251_store_val(struct class_device* cd, const char*
> buf, size_t len) if (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -839,7 +843,8 @@ static ssize_t et61x251_show_i2c_reg(struct
> class_device* cd, char* buf) if
> (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -865,7 +870,8 @@ et61x251_store_i2c_reg(struct class_device* cd, const
> char* buf, size_t len) if (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -897,7 +903,8 @@ static ssize_t et61x251_show_i2c_val(struct
> class_device* cd, char* buf) if
> (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
> @@ -934,7 +941,8 @@ et61x251_store_i2c_val(struct class_device* cd, const
> char* buf, size_t len) if (mutex_lock_interruptible(_sysfs_lock))
> return -ERESTARTSYS;
>
> - cam = video_get_drvdata(to_video_device(cd));
> + cam = video_get_drvdata(container_of(cd, struct video_device,
> + class_dev));
> if (!cam) {
> mutex_unlock(_sysfs_lock);
> return -ENODEV;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] v4l: fix build error for et61x251 driver
This fixes a kernel build problem and
should make it into 2.6.23, I think.
Regards,
Andreas
--
Get rid of some v4l1 remainders to avoid kernel build errors if
V4L1_COMPAT is not selected:
drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_:
drivers/media/video/et61x251/et61x251_core.c:718: error: implicit
declaration of to_video_device
Fix as suggested by Luca Risolia <[EMAIL PROTECTED]>
Signed-off-by: Andreas Herrmann <[EMAIL PROTECTED]>
---
drivers/media/video/et61x251/et61x251_core.c | 24
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/media/video/et61x251/et61x251_core.c
b/drivers/media/video/et61x251/et61x251_core.c
index 585bd1f..a3ee968 100644
--- a/drivers/media/video/et61x251/et61x251_core.c
+++ b/drivers/media/video/et61x251/et61x251_core.c
@@ -715,7 +715,8 @@ static ssize_t et61x251_show_reg(struct class_device* cd,
char* buf)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -739,7 +740,8 @@ et61x251_store_reg(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -771,7 +773,8 @@ static ssize_t et61x251_show_val(struct class_device* cd,
char* buf)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -803,7 +806,8 @@ et61x251_store_val(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -839,7 +843,8 @@ static ssize_t et61x251_show_i2c_reg(struct class_device*
cd, char* buf)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -865,7 +870,8 @@ et61x251_store_i2c_reg(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -897,7 +903,8 @@ static ssize_t et61x251_show_i2c_val(struct class_device*
cd, char* buf)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
@@ -934,7 +941,8 @@ et61x251_store_i2c_val(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(_sysfs_lock);
return -ENODEV;
--
1.5.3
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] v4l: fix build error for et61x251 driver
This fixes a kernel build problem and
should make it into 2.6.23, I think.
Regards,
Andreas
--
Get rid of some v4l1 remainders to avoid kernel build errors if
V4L1_COMPAT is not selected:
drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_:
drivers/media/video/et61x251/et61x251_core.c:718: error: implicit
declaration of to_video_device
Fix as suggested by Luca Risolia [EMAIL PROTECTED]
Signed-off-by: Andreas Herrmann [EMAIL PROTECTED]
---
drivers/media/video/et61x251/et61x251_core.c | 24
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/media/video/et61x251/et61x251_core.c
b/drivers/media/video/et61x251/et61x251_core.c
index 585bd1f..a3ee968 100644
--- a/drivers/media/video/et61x251/et61x251_core.c
+++ b/drivers/media/video/et61x251/et61x251_core.c
@@ -715,7 +715,8 @@ static ssize_t et61x251_show_reg(struct class_device* cd,
char* buf)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -739,7 +740,8 @@ et61x251_store_reg(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -771,7 +773,8 @@ static ssize_t et61x251_show_val(struct class_device* cd,
char* buf)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -803,7 +806,8 @@ et61x251_store_val(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -839,7 +843,8 @@ static ssize_t et61x251_show_i2c_reg(struct class_device*
cd, char* buf)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -865,7 +870,8 @@ et61x251_store_i2c_reg(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -897,7 +903,8 @@ static ssize_t et61x251_show_i2c_val(struct class_device*
cd, char* buf)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -934,7 +941,8 @@ et61x251_store_i2c_val(struct class_device* cd, const char*
buf, size_t len)
if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
--
1.5.3
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
Hacked-by: Luca Risolia [EMAIL PROTECTED]
On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote:
This fixes a kernel build problem and
should make it into 2.6.23, I think.
Regards,
Andreas
--
Get rid of some v4l1 remainders to avoid kernel build errors if
V4L1_COMPAT is not selected:
drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_:
drivers/media/video/et61x251/et61x251_core.c:718: error: implicit
declaration of to_video_device
Fix as suggested by Luca Risolia [EMAIL PROTECTED]
Signed-off-by: Andreas Herrmann [EMAIL PROTECTED]
---
drivers/media/video/et61x251/et61x251_core.c | 24
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/media/video/et61x251/et61x251_core.c
b/drivers/media/video/et61x251/et61x251_core.c index 585bd1f..a3ee968
100644
--- a/drivers/media/video/et61x251/et61x251_core.c
+++ b/drivers/media/video/et61x251/et61x251_core.c
@@ -715,7 +715,8 @@ static ssize_t et61x251_show_reg(struct class_device*
cd, char* buf) if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -739,7 +740,8 @@ et61x251_store_reg(struct class_device* cd, const char*
buf, size_t len) if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -771,7 +773,8 @@ static ssize_t et61x251_show_val(struct class_device*
cd, char* buf) if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -803,7 +806,8 @@ et61x251_store_val(struct class_device* cd, const char*
buf, size_t len) if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -839,7 +843,8 @@ static ssize_t et61x251_show_i2c_reg(struct
class_device* cd, char* buf) if
(mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -865,7 +870,8 @@ et61x251_store_i2c_reg(struct class_device* cd, const
char* buf, size_t len) if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -897,7 +903,8 @@ static ssize_t et61x251_show_i2c_val(struct
class_device* cd, char* buf) if
(mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
@@ -934,7 +941,8 @@ et61x251_store_i2c_val(struct class_device* cd, const
char* buf, size_t len) if (mutex_lock_interruptible(et61x251_sysfs_lock))
return -ERESTARTSYS;
- cam = video_get_drvdata(to_video_device(cd));
+ cam = video_get_drvdata(container_of(cd, struct video_device,
+ class_dev));
if (!cam) {
mutex_unlock(et61x251_sysfs_lock);
return -ENODEV;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
On Fri, 14 Sep 2007, Luca Risolia wrote: Hacked-by: Luca Risolia [EMAIL PROTECTED] On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote: This fixes a kernel build problem and should make it into 2.6.23, I think. Regards, Andreas -- Get rid of some v4l1 remainders to avoid kernel build errors if V4L1_COMPAT is not selected: drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_: drivers/media/video/et61x251/et61x251_core.c:718: error: implicit declaration of to_video_device Fix as suggested by Luca Risolia [EMAIL PROTECTED] This patch is really ugly. Why can't the to_video_device() macro be used? Just move it to a place where it's usable! IOW, what's wrong with the *much* simpler patch below? That to_video_device() macro has absolutely _nothing_ to do with CONFIG_VIDEO_V4L1_COMPAT, as far as I can tell! Linus --- diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index d62847f..17f8f3a 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -337,6 +337,9 @@ void *priv; struct class_device class_dev; /* sysfs */ }; +/* Class-dev to video-device */ +#define to_video_device(cd) container_of(cd, struct video_device, class_dev) + /* Version 2 functions */ extern int video_register_device(struct video_device *vfd, int type, int nr); void video_unregister_device(struct video_device *); @@ -354,11 +357,9 @@ extern int video_usercopy(struct inode *inode, struct file *file, int (*func)(struct inode *inode, struct file *file, unsigned int cmd, void *arg)); - #ifdef CONFIG_VIDEO_V4L1_COMPAT #include linux/mm.h -#define to_video_device(cd) container_of(cd, struct video_device, class_dev) static inline int __must_check video_device_create_file(struct video_device *vfd, struct class_device_attribute *attr) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] v4l: fix build error for et61x251 driver
On Friday 14 September 2007 02:09:01 Linus Torvalds wrote: On Fri, 14 Sep 2007, Luca Risolia wrote: Hacked-by: Luca Risolia [EMAIL PROTECTED] On Friday 14 September 2007 00:27:17 Andreas Herrmann wrote: This fixes a kernel build problem and should make it into 2.6.23, I think. Regards, Andreas -- Get rid of some v4l1 remainders to avoid kernel build errors if V4L1_COMPAT is not selected: drivers/media/video/et61x251/et61x251_core.c: In et61x251_show_: drivers/media/video/et61x251/et61x251_core.c:718: error: implicit declaration of to_video_device Fix as suggested by Luca Risolia [EMAIL PROTECTED] This patch is really ugly. Why can't the to_video_device() macro be used? Just move it to a place where it's usable! IOW, what's wrong with the *much* simpler patch below? There's nothing wtong in my opinion. I do not know the exact reason why Mauro moved to_video_device() into CONFIG_VIDEO_V4L1_COMPAT. Pheraps he can give more details about this change. That to_video_device() macro has absolutely _nothing_ to do with CONFIG_VIDEO_V4L1_COMPAT, as far as I can tell! Linus --- diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index d62847f..17f8f3a 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -337,6 +337,9 @@ void *priv; struct class_device class_dev; /* sysfs */ }; +/* Class-dev to video-device */ +#define to_video_device(cd) container_of(cd, struct video_device, class_dev) + /* Version 2 functions */ extern int video_register_device(struct video_device *vfd, int type, int nr); void video_unregister_device(struct video_device *); @@ -354,11 +357,9 @@ extern int video_usercopy(struct inode *inode, struct file *file, int (*func)(struct inode *inode, struct file *file, unsigned int cmd, void *arg)); - #ifdef CONFIG_VIDEO_V4L1_COMPAT #include linux/mm.h -#define to_video_device(cd) container_of(cd, struct video_device, class_dev) static inline int __must_check video_device_create_file(struct video_device *vfd, struct class_device_attribute *attr) Best regards Luca Risolia - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
