Hi Mauro, > > > I am trying out linux-uvc with version 2.6.18-rc6-git3 and it reports > > > unknown symbols for video_get_drvdata and video_set_drvdata. Based on > > > another patch for this problem [1] I have fixed this for uvc. The fix > > > works for me but please bear in mind that I have little kernel > > > programming experience and thus it could be wrong. > > > > Thanks for the patch. It looks overly complicated to me, and it seems > > that video_get_drvdata/video_set_drvdata should not have been removed, > > but modified to use dev_get_drvdata/dev_set_drvdata instead. > > > > Mauro, could you have a look at that ? > > Sure! Can you email it to me? I'm not subscribed on linux-uvc-devel.
You can find it at https://lists.berlios.de/pipermail/linux-uvc-devel/attachments/20060915/4e138fd2/attachment-0001.bin > > Why have those been removed ? I know > > they were marked with OBSOLETE_OWNER, but that's becaused they accessed > > the video_device structure's priv field, marked for removal. > > > > /* dev->driver_data will be used instead some day. > > * Use the video_{get|set}_drvdata() helper functions, > > * so the switch over will be transparent for you. > > * Or use {pci|usb}_{get|set}_drvdata() directly. */ > > void *priv; > > > > Shouldn't video_{get|set}_drvdata() have been kept ? > > This stuff just add more complexity and makes the code more opaque, and > adds this additional pointer... I agree that video_device->priv should go. But it should be replaced by something else, namely dev->driver_data. > For most cases, it will just turn on something like this (from > cx88-video.c): > > static ssize_t > video_read(struct file *file, char __user *data, size_t count, loff_t > *ppos) > { > struct cx8800_fh *fh = file->private_data; This is an entirely different thing. file->private_data contains a *per-open* pointer to private data. video_{get|set}_drvdata was used to retrieve private data global to the device. Have a look at the changes to em2820_v4l2_open in http://www.linuxtv.org/downloads/video4linux/patches/2.6.14/v4l_813_replaced_obsolete_video_get_drvdata_and_video_set_drvdata.patch Do you really want to require *every* video driver to maintain a global list of devices ? Think about overhead, locking issues, ... Drivers should use file->private_data to point to *per-open* private data, not per-device data. per-device data should be stored in device->driver_data instead of video_device->priv (device->driver_data was made for exactly that purpose), and the helper functions video_{get|set}_drvdata should be kept, calling dev_{get|set}_drvdata instead of accessing video_device->priv. Things are quite simple. Video devices are class devices, so per-device driver-private data is stored in device->driver_data, and per-open driver-private data is stored in file->file_private. That's how the driver model has been designed since the beginning of Linux 2.6. I can't imagine requiring *every* driver to maintain a global list of devices just to store private data. Laurent Pinchart _______________________________________________ Linux-uvc-devel mailing list [email protected] https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
