lkml.org 
[lkml]   [2007]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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 <luca.risolia@studio.unibo.it>
> > >
> > > 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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-09-14 09:51    [from the cache]
©2003-2011 Jasper Spaans