On Sat May 5 2012 13:14:10 Sakari Ailus wrote: > Hi Hans, > > On Sat, May 05, 2012 at 12:55:01PM +0200, Hans Verkuil wrote: > > On Sat May 5 2012 12:25:45 Sakari Ailus wrote: > > > From: Rémi Denis-Courmont <remi@xxxxxxxxxx> > > > > > > V4L2 uses the enum type in IOCTL arguments in IOCTLs that were defined until > > > the use of enum was considered less than ideal. Recently Rémi Denis-Courmont > > > brought up the issue by proposing a patch to convert the enums to unsigned: > > > > > > <URL:http://www.spinics.net/lists/linux-media/msg46167.html> > > > > > > This sparked a long discussion where another solution to the issue was > > > proposed: two sets of IOCTL structures, one with __u32 and the other with > > > enums, and conversion code between the two: > > > > > > <URL:http://www.spinics.net/lists/linux-media/msg47168.html> > > > > > > Both approaches implement a complete solution that resolves the problem. The > > > first one is simple but requires assuming enums and __u32 are the same in > > > size (so we won't break the ABI) while the second one is more complex and > > > less clean but does not require making that assumption. > > > > > > The issue boils down to whether enums are fundamentally different from __u32 > > > or not, and can the former be substituted by the latter. During the > > > discussion it was concluded that the __u32 has the same size as enums on all > > > archs Linux is supported: it has not been shown that replacing those enums > > > in IOCTL arguments would break neither source or binary compatibility. If no > > > such reason is found, just replacing the enums with __u32s is the way to go. > > > > > > This is what this patch does. This patch is slightly different from Remi's > > > first RFC (link above): it uses __u32 instead of unsigned and also changes > > > the arguments of VIDIOC_G_PRIORITY and VIDIOC_S_PRIORITY. > > > > > > Signed-off-by: Rémi Denis-Courmont <remi@xxxxxxxxxx> > > > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx> > > > --- > > > include/linux/videodev2.h | 46 ++++++++++++++++++++++---------------------- > > > 1 files changed, 23 insertions(+), 23 deletions(-) > > > > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > > index 5a09ac3..585e4b4 100644 > > > --- a/include/linux/videodev2.h > > > +++ b/include/linux/videodev2.h > > > @@ -292,10 +292,10 @@ struct v4l2_pix_format { > > > __u32 width; > > > __u32 height; > > > __u32 pixelformat; > > > - enum v4l2_field field; > > > + __u32 field; > > > > One suggestion: add a comment like this: > > > > __u32 field; /* refer to enum v4l2_field */ > > > > This keeps the link between the u32 and the enum values. > > > > Note that the DocBook documentation also has to be updated. And v4l2-compat-ioctl32 as well! :-) That too has enums in the compat structs. Regards, Hans > > > > Looks good to me otherwise. > > Thanks for the comments. I'll make the above changes. > > Cheers, > > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
