Re: [PATCH] v4l: fix build error for et61x251 driver

[email protected]
Newsgroups gmane.linux.usb.devel,gmane.comp.video.video4linux,gmane.linux.kernel
Message-ID <20070914074852.GA5598@devil>
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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.